Don't understand why C program crashes, pointer array of strings

Question

I'm having trouble with this for loop and I don't understand why it crashes. I'm trying to read an input list of 20 names in "first name last name" format and storing them as a string in "last name, first name". Duplicates should not be stored into the array pointer.

When I comment out the malloc and compare loop, apparently there is some issue with the address staying the same, so that *ary returns the same value as *walker. The filePtr works and the strcpy and strcat functions have no issues. Also, removing the first printf also causes the program to crash, even though removing it doesn't seem it should have any real effect besides output.

FILE *filePtr = fopen ("input.txt","r");
int size = 20;
char **ary;
char **walker;
char **end;
int strsize = 0;
char firstname[30] = {0};
char lastname[30] = {0};
char *fullname;
ary = calloc (size, sizeof(char *));
printf("%d\n",sizeof(pAry));
for ( walker = ary ; *walker < (*end = *ary + size) ; walker++)
{
    fscanf(filePtr," %s",firstname);
    fscanf(filePtr," %[^\n]",lastname);
    strsize = strlen(firstname) + strlen(lastname) + 3;
    fullname = malloc (strsize * sizeof(char));
    strcpy(fullname,lastname);
    strcat(fullname,", ");
    strcat(fullname,firstname);
    for ( compare = 0 ; compare < walker ; compare++)
    {
        if(strcmp(fullname,*(ary + compare)) != 0)
        {
            diff = 0;
        }
    }
    if (diff)
    {
        strncpy(*walker,fullname,strsize);
        printf("%s\n",*walker);
    }
    free(fullname);
}

Answer 1

The outer loop shall loop over all entries of ary, so the end condition shall test for walker being (at) the end.

No dereferencing is needed here:

for (walker = ary; walker < (end = ary + size); walker++)

---

The test loop for duplicates does compare againt absolute pointers values, the initialisation of compare to 0 implies relative comparsion so this line

compare < walker;

should be

compare < (walker - ary);

Substracting two pointers returns an integer, its size depends on the size of a pointer, which differs depending on compiler and/or system. To get around this uncertainty the integer type ptrdiff_t had been introduced to be guaranteed to hold any pointer difference.

So compare shall be declare:

ptrdiff_t compare;

---

strcmp() returns 0 if the strings to be compared are equal, so setting diff to 0 on inequality is wrong.

You might like to use the following statement to set diff:

    diff = strcmp(fullname,*(ary + compare));

This sets diff to 0 (false) if the two string a equal (not *diff*erent).

Also comparision shall stop after a dupe had been found.

    if (!diff)
    {
      break;
    }

Finally diff needs to be (re-)initialised for every iteration.

---

Instead of

    strncpy(*walker, fullname, strsize);

do

    *walker = fullname;

as fullname referrs to the freshly allocated memory and needs to be stored, as overwritten in the next iteration.

The line free()ing fullname

  free(fullname);

needs to be removed then.

---

Putting all this together you get:

...

for (walker = ary; walker < (end = ary + size); walker++)
{
  ...

  {
    int diff = 1;
    for (ptrdiff_t compare = 0; compare < (walker - ary); compare++)
    {
      diff = strcmp(fullname, *(ary + compare));       
      if (!diff)
      {
        break;
      }
    }

    if (diff)
    {
      *walker = fullname;
      printf("%s\n", *walker);
    }
  }
}