This code return wrong first pointer in a array of pointers

Question

This is my code

#include <stdio.h>

int main()
{
    char *fruit[] = {
        "Water",
        "banana",
        "pear",
        "apple",
        "coconut",
        "grape",
        "blueberry"
    };
    int x;
    int g;

    for(x=0;*(fruit+x)!='\0';x++)
    {
        for (g=0; *(*fruit)++; g++)
        {
            putchar(*(*(fruit+x)+g));
        }
        putchar('\n');
    }

    return(0);
}

And this is what the code returns...

aeaa
banana
pear
apple
coconut
grape
blueberry
Program ended with exit code: 0

I don't understand what I'm doing wrong... Why the first point goes wrong and the others pointers display as I expect.

Can anyone please explain? I'm using Xcode.

Answer 1

To understand what is going on you should place a breakpoint on your first for line, run the code till it stops and then step through watching the values of fruit, x & g. If you do this you'll notice something strange - after the first time around the outer loop the first element of fruit changes to banana (the remaining entries stay the same, so you now have two bananas).

Do this now and watch. If you can't use the debugger stop now and figure it out.

So what is happening?

First let's look at what your code outputs:

ae?aa (? is actually upside down on my run)
banana
etc.

Where could those characters be coming from? Well if the compiler packs your strings into memory, remembering that each string ends in a null byte which we will represent by ? then packed into memory you have:

Water?banana?pear?apple

etc. Your program is printing out 5 characters, the number in Water, and they are the 2nd, 4th, 6th, 8th and 10th in memory...

So now we know what we're seeing, why are we seeing it?

Let's look at fruit, it is a 1-dimensional array of char * - pointers into memory containing the characters in the strings. If we look at the first few pointers in this array when I run your code they are (actual values may differ):

0x100000e24
0x100000e2a
0x100000e31

Looking at just the last two digits, and converting from hexadecimal, we have 36, 42, 49. The first difference is 6, the number of bytes needed to store "Water" (including the trailing null byte), the next is 7, the number of bytes needed to store "banana", and so it goes on.

Now let's look at your first for loop:

for(x=0; *(fruit+x) != '\0'; x++)

Now *(fruit+x) is the same as writing fruit[x], so in each iteration of the loop you are looking at an element of fruit starting with the first (index 0). Now fruit contains values of type char * but you are comparing the value to a char - they are not the same thing!

When C constructs an array with unspecified bounds (the empty [] in the declaration of fruit) from a literal as you've done here it does not add anything after the last element of the array to indicate there are no more elements. There are two common ways to deal with this: you can calculate the number of elements using two calls to sizeof (see @Lundin's answer); or you can use a sentinel - a value which will not occur in the array otherwise. For arrays of pointer values the standard sentinel is NULL - the pre-declared pointer to nothing. So to make this part of your code work we change the code to:

char *fruit[] =
{
  "Water",
  "banana",
  "pear",
  "apple",
  "coconut",
  "grape",
  "blueberry",
  NULL
};

int x;
int g;

for(x=0; *(fruit+x) != NULL; x++)

This loop will now set x to the values 0 through 6.

Now let's look at your inner loop:

for (g=0; *(*fruit)++; g++)

To reduce the stars circling our heads, let's replace *first by first[0]:

for (g=0; *(fruit[0])++; g++)

So *(fruit[0])++:

1. Obtains the value in the first element of the array (fruit[0]), which is a pointer (char *) to the first character of the string "Water";
2. Indirects via that pointer (*(fruit[0]) to obtain a single character, which is W;
3. Implicitly compares that character to the null byte - the test part of the for; and
4. Increments the pointer stored in the first element of the array

So after the first iteration of this loop the fruit array contains (assuming values as above):

0x100000e25
0x100000e2a
0x100000e31

The first element has now changed to point at the a in "Water". As you are reading the array to print it you are also changing it - probably not a good idea.

Also note that this loop always references the first element of the array, it never moves past it however many iterations of the outer loop occur - this loop is not stepping through the elements of fruit either - to do that you would need to reference x somewhere, which you do not in the for itself, but you do in the body:

putchar(*(*(fruit+x)+g))

which re-written using indexing is:

putchar(fruit[x][g])

which outputs the g'th character of the x'th element of fruit. This would make sense if the array fruit was not being modified, however as we've just determined the first element of fruit now references the a in "Water", x & g are both zero, so this outputs a and not W as you hoped.

Now consider the second iteration, the for loop examines the a and find it is not the null byte, so fruit[0] is incremented to reference the t in "Water" and g is incremented to 1. Now the putchar looks up the first element of fruit, which is referencing "ter..." having been incremented twice, g is 1 so the second character is selected, which is e, and that is output.

In each iteration you are incrementing the pointer in fruit[0] and incrementing g, so the sum increments by 2 each time and the putchar outputs the 2nd, 4th, 6th, 8th and 10th characters in memory... As the strings follow each other you get a, e and the null byte from "Water" and two a's from "banana".

As your program continues fruit[0] marches through memory but as the putchar uses x and so references the unchanged pointers in fruit[1] onwards the remaining strings, by luck, print out correctly. If you don't add the sentinel to the array the outer loop keeps marching through memory until a zero byte is found - so you may get garbage printed after blueberry as well.

So how do you fix this?

Well you should never have been altering fruit[0] in there first place, and you should be referencing x in your inner for to step through the array.

Keeping close to your original code a solution is to copy the pointer stored in fruit into a local variable and increment that to walk through the string:

char *fruit[] =
{  "Water",
   "banana",
   "pear",
   "apple",
   "coconut",
   "grape",
   "blueberry",
   NULL
};

int x;
int g;

for(x=0; *(fruit+x) != NULL; x++)
{
   char *p = *(fruit+x);  // copy the x'th pointer from fruit and store it in p
   for (g=0; *(p+g); g++) // step through the string incrementing p (not the x'th element of fruit)
   {
      putchar(*(p+g));
   }
   putchar('\n');
}

HTH

Answer 2

You forgot to NULL terminate your array:

char *fruit[] = {
    "Water",
    "banana",
    "pear",
    "apple",
    "coconut",
    "grape",
    "blueberry",
    NULL
};

You have been hit by undefined behavior.

Then use just puts like

for(x=0;fruit[x]!=NULL;x++)
   puts(fruit[x]);

Please compile with all warnings and debug info (perhaps gcc -Wall -g) and use the debugger (gdb probably) - run your code step by step and print or display some relevant variables (like x or fruit)

Answer 3

The code is needlessly complicated. Is there a reason why you can't simply do like this?

#include <stdio.h>

#define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(*arr))

int main()
{
    const char *fruit[] = 
    {
        "Water",
        "banana",
        "pear",
        "apple",
        "coconut",
        "grape",
        "blueberry"
    };


    for(int i=0; i<ARRAY_SIZE(fruit); i++)
    {
        puts(fruit[i]);
    }

    return 0;
}