0

In C, I want to check a given array of chars for an arbitrary letter, and change it according to what it is. For example, the characters "a" or "A" would be changed to "4"(the character representing 4). This is a coding excercise for me :)

The code is as follows:

#include <stdio.h>
#include <string.h>
#include <assert.h>
#include <zlib.h>

#define NUM_BUFFERS 8
#define BUFFER_LENGTH 1024

char buffArrays[NUM_BUFFERS][BUFFER_LENGTH];

int main(int argc, const char* arg[v])
{
    const char a[] = "a";

    gzFile file;
    file = gzopen("a.txt", "rb"); //contains 8 lines of 1024 'a's
    int counter = 0;
    while(counter < NUM_BUFFERS)
    {
        gzread(file, buffArrays[counter], BUFFER_LENGTH - 1);
        counter++;
    }
    counter = 0;
    while(counter < NUM_BUFFERS)
    {
        int i = 0;
        for( i; i < BUFFER_LENGTH; i++ )
        {
            int *changed = &buffArrays[counter][i];
            if( memcmp(&a, changed, 1) == 0 )
                printf("SUCCESS\n");
        }
        counter++;
    }
    gzclose(file);
    return 0;
}

This code never reaches the "SUCCESS" part. This says to me that either

(1) the value of changed is not pointing to the correct thing (2) the pointer &a is incorrect (3) I am completely wrong and it is something else

Any help would be appreciated.

Sam P
  • 453
  • 6
  • 19

4 Answers4

1

The statement

bufferArrays[counter] = "a";

is not legal. It assigns a pointer to a single char and should give a compiler error (or at least a warning). Instead try

bufferArrays[counter] = 'a';

Also, in the while loops (both of them) you do not increase counter and so loop over the same index over and over forever.

Edit: Further problems

The condition where you do the comparison is flawed as well:

memcmp(&a, changed, 4)

The above doesn't compare pointers, it compares the contents of what the pointers point to, and you compare four bytes while the contents is only a single byte. Besides, you can't compare the pointers, as they will be different; The contents of the variable a is stored at a different location than that of the contents of bufferArrays[counter][i].

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
1

This:

bufferArrays[counter] = "a"; //all the buffers contain one "a"

is wrong, since bufferArrays[counter] is not a character pointer but a character array. You need:

strcpy(bufferArrays[counter], "a");

Also, you don't show readTOmodify, so that part is a bit hard to understand.

Further, strings are best compared with strcpy(), which compares character-by-character and stops at the terminating '\0'. You use memcmp(), and I don't understand the reason for the 4 which is the number of bytes you're comparing.

unwind
  • 391,730
  • 64
  • 469
  • 606
  • I was unsure how many bytes I need to compare to check a POSIX standard char. I put 4 just in case. check new edits – Sam P Sep 19 '12 at 06:52
  • Also, I'm looking for commentary on my pointering rather than this. It is not my code verbatim, not even the length of the buffers is the same. – Sam P Sep 19 '12 at 07:01
1

1) bufferArrays[counter] = "a"; //all the buffers contain one "a"

This is not ok, you have to use strcpy to copy strings: 

strcpy(bufferArrays[counter],"a"); //all the buffers contain one "a"

2)

#define BUFFER_LENGTH 1

Here's a problem. Buffer length should be at least 2 if you want to store just one char (for the extra null-termination).

3) In both of your loops, you never change counter, which leads to infinite loop.

Where's your code? I don't see any function surrounding it.

EDIT: To assign you can also use: 

while(counter < NUM_BUFFERS)
{
    bufferArrays[counter][0] = 'a'; //all the buffers contain one "a"
    counter++;
}

In any case, you have to have Buffer length as 2 if you want use it as a C-string.

P.P
  • 117,907
  • 20
  • 175
  • 238
  • changed function. This is not my code verbatim, but just a snippet. – Sam P Sep 19 '12 at 06:51
  • @SamP Still the `BUFFER_LENGTH` and `strcpy` issues are there. – P.P Sep 19 '12 at 06:52
  • can I use bufferArrays[counter] = 'a'; ? In my real code it is being read in by gzread from the libz library, but I didn't want any commentary on that but it looks like I've just caused confusion. – Sam P Sep 19 '12 at 06:55
  • No, because `bufferArrays` is a 2D array. If you just want to copy one char then you can use: `bufferArrays[counter][0]='a';` and loop through. – P.P Sep 19 '12 at 06:57
  • You still compare 4 bytes and there's no function...Can you post a compiling code even if it's not working as expected. – P.P Sep 19 '12 at 07:07
1

Two things.

The following assigns the value 0x61 or 'a' to the character string. 

const char a[] = 'a';

You probably rather meant to write

const char a = 'a'; /* assign a character to a character */

or 

const char a[] = "a"; /* assign a string to a string */

The next thing is with the following statement. Hereby you assign a pointer to an int with the memory address of a char. Which invokes undefined behavior as you are reading over the bounds of your valid memory in the next statement.

int *changed = &bufferArrays[counter][i];

Hereby you compare the first four bytes starting from both addresses. Both variables are only one byte wide.

if( memcmp(&a, changed, 4) == 0 )

If you only want to know whether there is an 'a' in some of your buffer, why don't you just.

int i, j;
for (i = 0; i < NUM_BUFFERS; i++)
  for (j = 0; j < BUFFER_LENGTH; j++)
    if (bufferArrays[i][j] == 'a') printf("got it!\n");
moooeeeep
  • 31,622
  • 22
  • 98
  • 187