C pointer and dynamic alloc

Question

I'm studying the C language for just 2 weeks now and I'm facing problems with the dynamic allocs and the pointers, I'm a little bit confused about the following code.

I try to understand how the read function work but the *m confuse me. I just can't find a way to use the *m parametre, also the

if((*h = (struct inhabitant *)malloc(sizeof(struct inhabitant) * (*m))) == NULL)

I'm just blown away.

Here's the code:

#include <stdlib.h>
#include "inhabitants.h"
#include "sort.h"

void read(FILE *s, struct inhabitant **h, int *m) {
  int i, ntok;
  struct inhabitant *tmph;

  ntok = fscanf(s, "%d", m);
  if(ntok != 1 || *m < 0) {
    fprintf(stderr, "Unable to read file.\n");
    exit(-1);
  }

  if((*h = (struct inhabitant *)malloc(sizeof(struct inhabitant) * (*m))) == NULL) {
    fprintf(stderr, "Unable to allocate space for inhabitants.\n");
    exit(-1);
  }

  tmph = *h;
  for(i = 0; i < (*m); ++i) {
    ntok = fscanf(s, "%d %s %s %d", &(tmph[i].distance), (char *)&(tmph[i].prenom), (char *)&(tmph[i].nom), (int *)&(tmph[i].zip));
    if(ntok != 4) {
      fprintf(stderr, "File wrongly formatted.\n");
      exit(-1);
    }
  }

}

int compare_inhabitants_by_distance(struct inhabitant *a, struct inhabitant *b) {
    if (a->distance > b->distance)
        return 1;
    else
        return 0;
  //à compléter
}

int compare_inhabitants_by_zipcode(struct inhabitant *a, struct inhabitant *b) {
  if (a->enum zipcode > b->enum zipcode)
        return 1;
    else
        return 0;
  //à compléter
}

void show(int n, struct inhabitant *a) {
  int i;
  for(i = 0; i < n; ++i) {
    printf("%d, %s, %s, %d\n", a[i].distance, a[i].prenom, a[i].nom, a[i].zip);
  }
}

void printout(FILE *s, int n, struct inhabitant *a) {
  int i;
  for(i = 0; i < n; ++i) {
    fprintf(s, "%d %s %s %d\n", a[i].distance, a[i].prenom, a[i].nom, a[i].zip);
  }
}

#define PERSONS_TO_SAVE_FILE_IN "evacuation_plan0.txt"
#define PERSONS_TO_SAVE_FILE_OUT "better_evacuation_plan0.txt"

int main(int argc, char **argv) {
  FILE *s;
  int n;
  /*For testing purpose :
  struct inhabitant inhabs[] = {
                                { 100, "Jean", "France", GUADELOUPE },
                                { 10, "Ameni", "Braminia", STBARTH },
                                { 12, "Mathieu", "Krister", GUADELOUPE },
                                { 23, "Hilaire  ", "Blanchi", STMARTIN }
                              };
  n = sizeof(inhabs) / sizeof(*inhabs);*/

  struct inhabitant *inhabs0;


  if((s = fopen(PERSONS_TO_SAVE_FILE_IN, "r")) == NULL) {
    fprintf(stderr, "Unable to open file.");
    exit(-1);
  }

  read(s, inhabs, )

  /*
  A compléter :
  - Lecture du fichier.
  - Tris.
  */

  if((s = fopen(PERSONS_TO_SAVE_FILE_OUT, "w+")) == NULL) {
    fprintf(stderr, "Unable to open file.");
    exit(-1);
  }
  printout(s, n, inhabs0);
  fclose(s);

  free(inhabs0);

  return EXIT_SUCCESS;
}

Your code is not an [MCVE]. – Basile Starynkevitch Oct 21 '17 at 08:27 — Basile Starynkevitch, Oct 21 '17 at 08:27

Basile Starynkevitch · Accepted Answer · 2017-10-21T09:08:54.020

In C, an assignment is some expression, and expressions are a simple (and very common) kind of statement.

So instead of

int a = 2+3;
if (a>4) {

you might code

int a;
if ((a=2+3) > 4) {

which has the same semantics.

So your if statement is similar to:

*h = (struct inhabitant *)malloc(sizeof(struct inhabitant) * (*m));
if (*h == NULL)

^{(using a complex if like in the original code, or splitting it into two statements, is a matter of readability and taste; however, be aware of sequence points)}

BTW, what follows in the original code is poor taste (and using exit(3) with -1 is also poor taste). You'll better use perror(3) or strerror(3) with errno(3) to explain why malloc failed. So I recommend then

  { // when *h is NULL because malloc failed
     fprintf(stderr, 
             "Unable to allocate space for %d inhabitants: %s\n", 
             *m, strerror(errno));
     exit(EXIT_FAILURE);
  }

and in your particular case (allocating an array) I would recommend using calloc(3) (to get some zero-initialized memory zone) instead of malloc.

BTW, I recommend changing the name of your read function. It can be confused with POSIX read.

Don't forget to compile with all warnings and debug info, so gcc -Wall -Wextra -g with GCC (read about Invoking GCC). Improve your code to get no warnings. Read the documentation of every function before using it. Learn to use the debugger gdb. Work hard to avoid undefined behavior, and be very scared of them.

Read also more about C dynamic memory allocation, pointer aliasing, virtual address space, memory leaks, garbage collection, reference counting. In addition of the gdb debugger, valgrind is sometimes useful too. Study also the source code of existing free software (e.g. on github), you'll learn a lot.

score 2 · Answer 2 · edited Nov 12 '21 at 12:39

In addition to the other error pointed out, your fscanf variables are wrong. You do no need to pass the address of prenum or nom as they are already pointers, e.g.

    for (i = 0; i < *m; ++i) {
        /* casts should not be necessary, prenom & nom already pointers */
        ntok = fscanf (s, "%d %s %s %d", &tmph[i].distance, (tmph[i].prenom),
                        (tmph[i].nom), &(tmph[i].zip));

Further, the allocation itself is somewhat clumsy. Use the variable name instead of the type for sizeof and there is no need to cast the return of malloc, e.g.

    /* allocate/validate space for 'm' inhabitants */
    if ((*h = malloc (sizeof **h * *m)) == NULL) {
        fprintf (stderr, "Unable to allocate space for inhabitants.\n");
        exit (-1);
    }

See: Do I cast the result of malloc?

Putting it altogether, your read function could be updated to something similar to:

void read (FILE *s, struct inhabitant **h, int *m)
{
    int i, ntok;
    struct inhabitant *tmph;

    ntok = fscanf (s, "%d", m);
    if (ntok != 1 || *m < 0) {
        fprintf (stderr, "Unable to read file.\n");
        exit (-1);
    }

    /* allocate/validate space for 'm' inhabitants */
    if ((*h = malloc (sizeof **h * *m)) == NULL) {
        fprintf (stderr, "Unable to allocate space for inhabitants.\n");
        exit (-1);
    }

    tmph = *h;
    for (i = 0; i < *m; ++i) {
        /* casts should not be necessary, prenom & nom already pointers */
        ntok = fscanf (s, "%d %s %s %d", &tmph[i].distance, (tmph[i].prenom),
                        (tmph[i].nom), &(tmph[i].zip));
        if (ntok != 4) {
            fprintf (stderr, "File wrongly formatted.\n");
            exit (-1);
        }
    }
}

note: you also have an apparent typo here in main(), read(s, inhabs, ) appears intended as read (s, &inhabs0); If you give read a meaningful return type, like perhaps int, then you could return 0 or 1 to indicate success/failure without automatically exiting your program from within the function.

C pointer and dynamic alloc

2 Answers2