I am trying to do a simple struct example but I don't understand why it is not printing the contents in C

Question

Here is my code. It reads the first header line and ignores it but everything after the first header line should be entered in the struct. But when I try to print out the struct array it doesn't work.

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

typedef struct User {
  char *name;
  char *process;
  int arrival;
  int duration;
}User;

User userList[100];
int numOfJobs = 0;
char header[20];

int main(int argc, char **argv) {
  scanf("%s %s %s %s", header, header, header, header);

  while(EOF != scanf("%s %s %d %d", userList[numOfJobs].name, userList[numOfJobs].process, &userList[numOfJobs].arrival, &userList[numOfJobs].duration)) {
numOfJobs++;
  }

  for(int i = 0; i < numOfJobs; i++) {
    printf("%s %s %d %d", userList[i].name, userList[i].process, userList[i].arrival, userList[i].duration);
  }

  return 0;
}

You have not allocated any memory for `struct` members `char *name;` and `char *process;` I am surprised it does not crash, when passing unallocated pointers to `scanf()`. Aside: don't test `EOF != ` instead test `4 ==` because if `scanf()` returns say `2` your test won't catch it. — Weather Vane, Sep 13 '20 at 21:10
@AnttiHaapala so I see. What difference does that make? They can't be used. They have no memory allocation, whether they are `NULL` or indeterminate. They are passed to `scanf` which expects there to be usable memory. But there isn't. — Weather Vane, Sep 13 '20 at 22:43
`while(scanf("%s %s %d %d", ...) == 4)`. `scanf()` can return less than the number of conversions requested on *matching failure*, yet you only check of an *input failure*. (in addition to your need to allocate for `name` and `process` before filling each.) — David C. Rankin, Sep 13 '20 at 22:49
I doubt whether `scanf("%s %s %s %s", header, header, header, header)` is well-defined. There isn't a sequence point between scanf format specifiers — M.M, Sep 14 '20 at 02:15

David C. Rankin · Accepted Answer · 2020-09-14T02:10:37.570

In struct User you declare two pointers, name and process, e.g.

typedef struct User {
    char *name;
    char *process;
    int arrival;
    int duration;
} User;

Before you can make use of either, they must point to valid storage. If you intent to copy character strings to them, then you must allocate blocks of storage sufficient to hold each string, assign the starting address of a block to each pointer so they point to valid memory, and then copy your string to the allocated block which you can then reference through the pointer.

The simplest way to make it all work is to first read name and process into aa temporary character array so that you can obtain the length of the string you need to store, then allocate storage (+1 for the nul-terminating character) and copy the string to the allocated block.

You cannot build any type of robust input routing around scanf(). It is full of pitfalls for the new C programmer. Instead, all users input should be done with fgets() to ensure you consume a complete line of user-input each time and then parrse the values you need from the buffer filled with fgets() using sscanf(). That way you can validate (1) the read of the input; and (2) parsing the information from the input separately. Most importantly, subsequent reads are not thwarted by a matching failure leaving characters unread in your input buffer.

From your description, you want to read/discard the first line and that is what lead to your use of scanf("%s %s %s %s", header, header, header, header);. Don't do that. Just read the first line into a buffer with fgets() and ignore it. That way if you have less than 4 words on the first line, your codes isn't stuck blocking for additional input.

Putting it altogether, you would do:

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

#define MAXC 1024       /* if you need a constant, #define one (or more) */
#define MAXU 100

typedef struct User {
    char *name;
    char *process;
    int arrival;
    int duration;
} User;

int main (void) {

    char buf[MAXC];                 /* buffer to handle user-input */
    User userList[MAXU];            /* don't declare global, pass as parameter */
    int numOfJobs = 0;
    
    if (!fgets (buf, MAXC, stdin))  /* read/discard first line */
        return 1;
    
    while (numOfJobs < MAXU && fgets (buf, MAXC, stdin)) {  /* protect bounds, read lines */
        char tmpname[MAXC], tmpproc[MAXC];  /* temporary storage for name, process */
        /* split line with strings into tmpname, tmpproc */
        if (sscanf (buf, "%s %s %d %d", tmpname, tmpproc, 
                                        &userList[numOfJobs].arrival,
                                        &userList[numOfJobs].duration) == 4) {
            
            size_t len = strlen (tmpname);  /* get lenth of tmpname */
            /* allocate / validate storage for name */
            if (!(userList[numOfJobs].name = malloc (len + 1))) {
                perror ("malloc-name");
                break;
            }   /* copy tmpname to name */
            memcpy (userList[numOfJobs].name, tmpname, len + 1);
            
            len = strlen (tmpproc);         /* get length of tmpproc */
            /* allocate / validate storage for process */
            if (!(userList[numOfJobs].process = malloc (len + 1))) {
                perror ("malloc-process");
                break;
            }   /* copy tmpproc to process */
            memcpy (userList[numOfJobs].process, tmpproc, len + 1);
        }
        numOfJobs++;    /* only increment on success */
    }
    
    for(int i = 0; i < numOfJobs; i++) {
        printf ("%s  %s  %d  %d\n", userList[i].name, userList[i].process, 
                                    userList[i].arrival, userList[i].duration);
        
        free (userList[i].name);    /* free all allocated memory */
        free (userList[i].process);
    }
}

(note: the use of numOfJobs < MAXU && fgets (buf, MAXC, stdin) as your read-loop condition where numOfJobs < MAXU protects your userList array bounds by preventing you from writing beyond the array if you input has more than MAXU (100) lines of information)

(note2: there is no need to declare global variables. Instead declare them in main() and pass as parameters to any function that needs them)

~~Since you haven't provided a sample input file, this code has not been tested, but based on your description it should work as intended.~~ I have made up a bit of sample data to check the code and show you how to check the memory usage for any errors:

Example Input File

$ cat dat/alloc_struct_ptrs.txt
my dog has fleas -- bummer
name_1  process-1.8  2542  1542
name_2  process-2.9  2982  2982
name_3  process-3.0  1124  3124
name_4  process-4.1  1118  4118
name_5  process-5.2  4323  5323
name_6  process-6.3  2761  6761
name_7  process-7.4  6914  7914
name_8  process-8.5  2022  8022
name_9  process-9.6  9539  9539

Example Use/Output

$ ./bin/alloc_struct_ptrs < dat/alloc_struct_ptrs.txt
name_1  process-1.8  2542  1542
name_2  process-2.9  2982  2982
name_3  process-3.0  1124  3124
name_4  process-4.1  1118  4118
name_5  process-5.2  4323  5323
name_6  process-6.3  2761  6761
name_7  process-7.4  6914  7914
name_8  process-8.5  2022  8022
name_9  process-9.6  9539  9539

Memory Use/Error Check

In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.

It is imperative that you use a memory error checking program to ensure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.

For Linux valgrind is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.

$ valgrind ./bin/alloc_struct_ptrs < dat/alloc_struct_ptrs.txt
==18317== Memcheck, a memory error detector
==18317== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==18317== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==18317== Command: ./bin/alloc_struct_ptrs
==18317==
name_1  process-1.8  2542  1542
name_2  process-2.9  2982  2982
name_3  process-3.0  1124  3124
name_4  process-4.1  1118  4118
name_5  process-5.2  4323  5323
name_6  process-6.3  2761  6761
name_7  process-7.4  6914  7914
name_8  process-8.5  2022  8022
name_9  process-9.6  9539  9539
==18317==
==18317== HEAP SUMMARY:
==18317==     in use at exit: 0 bytes in 0 blocks
==18317==   total heap usage: 20 allocs, 20 frees, 5,291 bytes allocated
==18317==
==18317== All heap blocks were freed -- no leaks are possible
==18317==
==18317== For counts of detected and suppressed errors, rerun with: -v
==18317== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Always confirm that you have freed all memory you have allocated and that there are no memory errors.

Look things over and let me know if you have further questions.

thanks for explaining! I could just never grasp the idea of how malloc works. I will try with your solution and will let you know if I have more questions! — Ell, Sep 14 '20 at 12:47
say if I were to change data in the struct, for an example, I want to change the duration of userList[1] from 4 to 5, do I just do `userList[1].duration = 5;` ? — Ell, Sep 14 '20 at 13:21
Sure, you can re-assign any of the values. The key here is this. Variables like `arrival` and `duration` have *automatic storage duration* (they are just type `int`) and so storage is reserved for them on the program stack at the time of declaration. The same is true for `name` and `process`, **BUT**, `name` and `process` are POINTERS. What do pointers hold as their value? The address in memory where the data lives. So even though `sizeof(a_pointer)` storage is reserved for each, that is simply enough storage to hold the address for something else. — David C. Rankin, Sep 14 '20 at 15:49
You allocate a block of memory and you assign the starting address for that new block to your pointer (that's all `mall0c()` returns, the starting address to a new blocks of memory),. Now your pointer holds the address of the new block as its value (i.e. the pointer points to the new memory address) and you can use those bytes as you would any other. Read these two answers: [Difference between char *pp and (char*) p?](https://stackoverflow.com/a/60519053/3422102) and [Pointer to pointer of structs indexing out of bounds(?)...](https://stackoverflow.com/a/60639540/3422102) on pointer basics. — David C. Rankin, Sep 14 '20 at 15:52

I am trying to do a simple struct example but I don't understand why it is not printing the contents in C

1 Answers1