0

I have recently encountered very weird behaviour with my c program. I gutted out most of the code just to isolate where the issue is happening for you guys.

The purpose of the program in its current state is to read in jobs from a txt file and print the contents to the screen. Here it is:

#include <stdio.h>

int main(){
    char* user;
    char process;
    int arrival;
    int duration;

    scanf("%*[^\n]"); //skipping header in the file.

    while(!feof(stdin))
    {
        scanf("%s\t%c\t%d\t%d\n", user, &process, &arrival, &duration);
        printf("%s\t%c\t%d\t%d\n", user, process, arrival, duration);
    }

    int x[5]; //<----- Causing the weird behaviour
}

The file I pipe into it is:

User    Process Arrival Duration
Jim     A       2       5
Mary    B       2       3
Sue     D       5       5
Mary    C       6       2

The issue that I'm running into is whenever I declare the int x array, whether it is at the bottom or top of my code, the while loop enters an infinite loop or the program seg faults.

It will go into an infinite loop or seg fault depending on the size I declare the array at. For example, if I declare the array to be size 5, it enters an infinite loop. However, if I declare the array to be size 2, it will seg fault.

I am compiling my program by running:

gcc -o myprog myprog.c

and I am piping the file by running:

cat jobs.txt | ./myprog

It is also worth noting that the program runs fine without the int x array declaration.

I am completely stumped as to what might be the issue, any thought?

  • 4
    You never initialize `user` to point at valid memory location -> the `scanf` (inside the `while` loop) invokes *undefined behavior* – UnholySheep Sep 30 '17 at 21:10
  • 1
    You will want to look at [**Why is while ( !feof (file) ) always wrong?**](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). Utilize the return of `scanf` instead. – David C. Rankin Sep 30 '17 at 21:19
  • `int x[2];` for 2,3,4 cause segfault. and `x[5]` cause weird behaviour – EsmaeelE Sep 30 '17 at 21:21

2 Answers2

2

You have an undefined behavior because of the pointer user which is not initialized. user must point to a memory area capable of storing what you want (see malloc() for example).

Gam
  • 684
  • 1
  • 8
  • 17
  • 2
    Or better yet, set a reasonable buffer size, (e.g. `char user[16] = "";`) and check the return of `scanf` for validation of conversion. – David C. Rankin Sep 30 '17 at 21:21
  • @DavidC.Rankin Can confirm this fixed the issue! Thanks. I found it very weird that the declaration of the array was somehow linked and exposed the odd behavior, but I'm glad it has been resolved. – Kohdy Nicholson Sep 30 '17 at 21:40
1

As you have found from the other answer, your initial problem is with char *user; which declares a character pointer that is uninitialized (e.g. it does not point to any valid block of memory). While you can dynamically allocate with malloc, calloc, or realloc, that may be an over-complication for your circumstance.

All you really need is to declare a character array sufficient to hold the user names. 16 chars is more than sufficient here.

Next while (!feof(fp)) is almost always wrong. (see link in my comment). scanf provides a return of the number of valid conversions that take place. In your case, with "%s..%c..%d..%d" (4-conversion specifiers), a return of 4 will indicate that no matching or input failure occurred. So instead of your feof check, just use the scanf return, e.g.

    scanf("%*[^\n]"); //skipping header in the file.

    while (scanf ("%15s %15s %d %d", user, process, &arrival, &duration) == 4)
        printf ("%s\t%c\t%d\t%d\n", user, *process, arrival, duration);

(note: for the simplified scanf format string "%15s %15s %d %d", arrival is declared as a character array (see below) and read as a string (to take advantage of leading white-space skipping) and then *arrival is used to pick off the character. This provides a bit more robust way to read your input in the event your input is space separated instead of *tab separated)

To avoid using magic numbers in your code (e.g. 16), declare a constant if you need one for the max characters in your arrays, e.g.

#define MAXC 16

int main (void) {
    char user[MAXC] = "", process[MAXC] = "";

note: it may look like "%15s %15s %d %d" breaks this rule, but sadly, there is no way to include a constant of variable in the scanf field width specifier that protects against reading more than 15 chars into your arrays -- remember, you must leave room for the final character -- the nul-terminating character.

Putting it altogether, you could do something like the following:

#include <stdio.h>

#define MAXC 16

int main (void) {
    char user[MAXC] = "", process[MAXC] = "";
    int arrival, duration;

    scanf("%*[^\n]"); //skipping header in the file.

    while (scanf ("%15s %15s %d %d", user, process, &arrival, &duration) == 4)
        printf ("%s\t%c\t%d\t%d\n", user, *process, arrival, duration);

    return 0;
}

Example Use/Output

$ ./bin/scanf_usr_arriv <dat/userprocarriv.txt
Jim     A       2       5
Mary    B       2       3
Sue     D       5       5
Mary    C       6       2

You may also want to consider reading all "lines of input" with a line-oriented input function like fgets and then calling sscanf on the resulting buffer to parse each of the variables. This has the benefit of allowing separate validation on the line being read, and then an independent validation on the parse of each variable from the line.

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

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85