How to sort an array of pointers to structures in C?

Question

Hey guys this is my first post here and i was wondering if any of you can help me figure out how to sort array of pointers to structures. Here's my code and here's my assignment if anyone is interested https://i.stack.imgur.com/Z9uUO.png.

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>
#include <math.h>
#include <stdlib.h>
#include <string.h>

#define MAX 50

struct address
{
   char name[50];
   char street[50];
   char citystate[50];
   char zip[20];
};

int main()
{
   struct address *ptr[50];
   struct address tptr;
   char buffer[80];
   int count = 0;

   for (int i = 0; i <MAX; i++)
   {
      ptr[i] = malloc(sizeof(struct address));

      if (gets(buffer)== NULL)
      {
         break;
      }
      else
      {
         strcpy((*ptr[i]).name, buffer);
         gets((*ptr[i]).street);
         gets((*ptr[i]).citystate);
         gets((*ptr[i]).zip);
         free(ptr[i]);
         count++;
      }
   }

   for (int x = 0; x<count; x++)
   {
      for (int y = 0; y<count - 1; y++)
      {
         if ((*ptr[y]).zip>(*ptr[y + 1]).zip)
         {
            tptr = ptr[y + 1];
            ptr[y + 1] = ptr[y];
            ptr[y] = tptr;
         }
      }
   }

   for (int i = 0; i < count; i++)
   {
      puts((*ptr[i]).name);
      puts((*ptr[i]).street);
      puts((*ptr[i]).citystate);
      puts((*ptr[i]).zip);
   }
}

start by indenting your code properly so that people (including yourself) can read it! — M.M, May 18 '15 at 04:23
actually, before that, post something that compiles. In the line `gets(buffer)`, `buffer` has not been declared. (Of course, `gets` is a mistake in itself too). — M.M, May 18 '15 at 04:24
sorry about the indentation and i did declare the buffer but i forgot to include it in here — Abbas Mamdani, May 18 '15 at 04:28
you call `free(ptr[i]);` which frees that address and then you go on to try and work with the addresses after freeing them... — M.M, May 18 '15 at 04:32
ok i got rid of that. But now it gives me the following errors uninitialized local variable 'tptr' used and also a value of type "struct address *" cannot be assigned to an entity of type "struct address" — Abbas Mamdani, May 18 '15 at 04:37
Suggest compiling with all warnings enabled. then would would see 1) several warnings about 'gets()' being depreciated/removed, so should not be used. 2) the lines like: 'tptr = ptr[y+1]' as an assignment of a 'struct addr' to a 'ptr to a struct addr' which is incompatible I.E. the code does not compile — user3629249, May 19 '15 at 22:23
The calls to gets() always get their input from stdin. If the user is supplying the input, then they need prompts as to when to input each field. If from a redirection to an input file, then each field will need to be on a separate line — user3629249, May 19 '15 at 22:35
this line: 'if ((*ptr[y]).zip>(*ptr[y + 1]).zip)' will (on the last execution of the loop) reference memory beyond the end of the ptr[] array. This results in undefined behaviour and can/will lead to a seg fault event — user3629249, May 19 '15 at 22:41
this kind of line: 'ptr[y + 1] = ptr[y];' (I suppose) is trying to copy the whole struct address. However, at best, it will copy one byte. suggest:' memcpy( &ptr[y+1], &ptr[y], sizeof( struct address );' — user3629249, May 19 '15 at 22:43
accessing the entries pointed to by ptr[] after that entry has been passed to free() is undefined behaviour and can/will lead to a seg fault event. suggest not calling free() (in a loop) until the end of the function. pre-initializing all the ptr[] entries to NULL will make the free() loop very simple, as it is ok to pass a pointer containing NULL to free() — user3629249, May 19 '15 at 22:45
1) strongly suggest using 'fgets()' rather than 'gets()' 2) always check (!=NULL) the returned value from malloc() (and family of functions) to assure the operation was successful. — user3629249, May 19 '15 at 22:52

score 2 · Answer 1 · edited May 23 '17 at 12:28

Problems I see in your code:

You are using gets. See another SO post that addresses the poblem of using gets. Use fgets instead.

if (fgets(buffer, sizeof(buffer), stdin)== NULL)

fgets((*ptr[i]).street, sizeof((*ptr[i]).street), stdin);
fgets((*ptr[i]).citystate, sizeof((*ptr[i]).citystate), stdin);
fgets((*ptr[i]).zip, sizeof((*ptr[i]).zip), stdin);

You are calling free on a pointer in the following line
```
free(ptr[i]);
```
and continue to use it later in the code. Remove that line. Add the code to free the allocated memory at the end of the function.
```
for (int i = 0; i < count; i++)
{
   free(ptr[i]);
}
```
You are assigning a struct address* to a variable of type struct address in the following line:
```
tptr = ptr[y + 1];
```
and you are assigning a struct address to a variable of type struct address* in the following line:
```
ptr[y] = tptr;
```
both of them can be fixed by changing the type of tptr to struct address*.
```
struct address *tptr;
```
The following code is not appropriate for comparing two strings:
```
 if ((*ptr[y]).zip>(*ptr[y + 1]).zip)
```
it only compares two pointer values. Use
```
 if (strcmp((*ptr[y]).zip,(*ptr[y + 1]).zip) > 0)
```

score 0 · Answer 2 · edited May 23 '17 at 10:26

Why do you want to free() the allocated memory once you fetch the data from the user?
You should free() the allocated memory at the end of the program, once after the printing is done.
struct address tptr; should be of type struct address *tptr; as you are assigning the value of the pointer.

Below are the changes: 1.

for (int i = 0; i <MAX; i++)
{
    ptr[i] = malloc(sizeof(struct address));

    if (gets(buffer)== NULL)
    {
        free(ptr[i]); /* free the memory if data not read */
        break;
    }
    else
    {
        strcpy((*ptr[i]).name, buffer);
        gets((*ptr[i]).street);
        gets((*ptr[i]).citystate);
        gets((*ptr[i]).zip);
        /* Do not free the mem here as you are bound to lose the data */
        count++;
    }
}

2.

for (int i = 0; i < count; i++)
{
    puts((*ptr[i]).name);
    puts((*ptr[i]).street);
    puts((*ptr[i]).citystate);
    puts((*ptr[i]).zip);
    free(ptr[i]); /* free the memory here */
}

PS : Using gets() is not a good idea. Check this Do not use gets for more info

David C. Rankin · Answer 3 · 2018-05-06T02:21:37.457

First, when dealing with pointer to structs, proper member access is with the -> operator (e.g. ptr[i]->street). As others have pointed out, do NOT use gets. It is no longer part of the C library and was deprecated because it was insecure. Use fgets or getline instead.

Next, (and this is a matter of form) avoid hardcoded numbers in your code. Use #define to set your values. This allows easy adjustment in a single place if values change.

With that, you were not far off with your code. Making only those changes, deleting the unnecessary math.h and adding strcmp for your sort, you can sort your structures in ascending order by zip as follows:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX 50
#define MAXL 80
#define MAXZ 20

struct address {
    char name[MAX];
    char street[MAX];
    char citystate[MAX];
    char zip[MAXZ];
};

int main ()
{
    struct address *ptr[MAX];
    struct address *tptr;
    char buffer[MAXL];
    int count = 0;

    for (int i = 0; i < MAX; i++) {
        ptr[i] = malloc (sizeof (struct address));

        if (fgets (buffer, MAXL, stdin) == NULL) {
            break;
        } else {
            strncpy (ptr[i]->name, buffer, MAXL);
            ptr[i]->name[MAX - 1] = 0;
            fgets (ptr[i]->street, MAX, stdin);
            fgets (ptr[i]->citystate, MAX, stdin);
            fgets (ptr[i]->zip, MAXZ, stdin);
            count++;
        }
    }

    for (int x = 0; x < count; x++) {
        for (int y = 0; y < (count - 1); y++) {
            if (strcmp (ptr[y]->zip, ptr[y + 1]->zip) > 0) {
                tptr = ptr[y + 1];
                ptr[y + 1] = ptr[y];
                ptr[y] = tptr;
            }
        }
    }

    for (int i = 0; i < count; i++) {
        printf ("\n%s", ptr[i]->name);
        printf ("%s", ptr[i]->street);
        printf ("%s", ptr[i]->citystate);
        printf ("%s", ptr[i]->zip);
    }
}

Input

$ cat dat/sortaddress.txt
some name
my street
my city, my state
55512
another name
another street
another city, another state
44412

Use/Output

$ ./bin/struct_address_sort <dat/sortaddress.txt

another name
another street
another city, another state
44412

some name
my street
my city, my state
55512

note: reading with fgets or getline will read the trailing newline and include that in the buffer. It is a good idea to strip the newline from your strings so you don't have miscellaneous newlines at the end of your data. There are many examples on StackOverflow.

Thank you very much for your help, the changes you suggested did the trick thank you. — Abbas Mamdani, May 19 '15 at 06:54
Glad to help. Like I said, you were not far off, just needed a little help handling the structure syntax and `strcmp` for the comparison with sort. You can use `qsort` (recommended for any significant sorting), but for simple examples, a manual sort is fine. — David C. Rankin, May 20 '15 at 03:45

How to sort an array of pointers to structures in C?

3 Answers3