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.sstatic.net/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);
   }
}

Answer 1

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.

Answer 2

Problems I see in your code:

1. 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);
   

2. 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]);
   }
   

3. 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;
   

4. 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)
   

Answer 3

1. Why do you want to free() the allocated memory once you fetch the data from the user?
2. You should free() the allocated memory at the end of the program, once after the printing is done.
3. 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