Need help solving Segmentation Fault that only occurs in linux

Question

I'm having a problem where my program - which runs fine in Windows - gives me a Segmentation Fault in Linux. I've searched through a lot of other questions that deal with Segmentation Faults and I think it has to do something with me not using malloc correctly.

The program is supposed to take a data file, which is assumed to be a list events, which have multiple tasks and each task has an associated amount of days needed to complete it. For example, the data file I am testing with looks like this:

1          15        3
1          27        6
1          36        4
2          15        5
3          18        4
3          26        1
4          15        2
4          26        7
4          27        7
5          16        4

with the left column being the event number, the middle column being the task number, and the right column being the amount of days needed. Below is my code.

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

int main(int argc, char *argv[])
{
    // Declare variables to be used
    FILE *in;
    FILE *out;
    char* inName;
    char* outName;
    int eventNum, taskNum, daysNum;
    int amtEvents, totalDays = 0;
    int **events;
    int **data, dataSize = 0;
    int currentEvent = 1;

    // Exit program if there are too few arguments entered
    if(argc < 2)
    {
        printf("Too few arguments.");
        return 0;
    }

    // Locate the -input and -output tags to determine
    // file names
    for(int i=0; i<argc; i++)
    {
        if(! strcmp(argv[i], "-input"))
        {
            inName = malloc(strlen(argv[i+1]) * (sizeof(char)));
            strcpy(inName, argv[i+1]);
        }

        if(!strcmp(argv[i], "-output"))
        {
            outName = malloc(strlen(argv[i+1]) * (sizeof(char)));
            strcpy(outName, argv[i+1]);
        }
    }

    if ((in = fopen(inName, "r")) == NULL){
        fprintf(stderr, "Input file usage: %s", inName);
        return 0;
    }


    out = fopen(outName, "w");

    while (!feof(in)) {
        if (fscanf(in, "%d %d %d", &eventNum, &taskNum, &daysNum) != 3)
            break;
        dataSize++;
    }

    in = fopen(inName, "r");

    // Store the data in the file into a
    // dynamically created 2D array
    data = malloc((dataSize) * sizeof(int));
    while (!feof(in)) {
        for(int i=0; i < dataSize+1; i++){
                data[i] = malloc(3 * sizeof(int));

                if (fscanf(in, "%d %d %d", &eventNum, &taskNum, &daysNum) != 3)
                    break;
                data[i][0] = eventNum;
                data[i][1] = taskNum;
                data[i][2] = daysNum;
        }
    }


    // Increment amtEvents for each unique event number
    amtEvents = 1;
    for(int i=0; i<dataSize; i++)
    {
        while(currentEvent != data[i][0])
        {
           amtEvents++;
           currentEvent++;
        }
    }

    // Create another 2D array to store data about
    // each event
    events = malloc(amtEvents * sizeof(int));
    for(int i=0; i < amtEvents+1; i++)
    {

        events[i] = malloc(3 * sizeof(int));
        for(int j=0; j<3; j++){
            events[i][j] = 0;
        }

    }


    for(int i=0; i < dataSize; i++)
    {

        events[data[i][0]-1][1]++;
        if(data[i][2] > events[data[i][0]-1][2])
            events[data[i][0]-1][2] = data[i][2];
    }
    // Header
    fprintf(out, "Project completion timetable\n");
    fprintf(out, "----------------------------------------\n");
    fprintf(out, "Event\tNum of tasks\tMax num.of days\n");
    fprintf(out, "-----\t------\t\t--------\n");


    for(int i=0; i<amtEvents; i++)
    {
        fprintf(out, "%d\t %d\t\t %d\n", i+1, events[i][1], events[i][2]);
        totalDays += events[i][2];
    }

    fprintf(out, "----------------------------------------\n");
    fprintf(out, "Total number of days to finish the project: %d", totalDays);
    free(inName);
    free(outName);
    free(data);
    free(events);
}

I'm not very familiar with using gdb, but I ran my program through it and this is what it told me:

Program received signal SIGSEGV, Segmentation fault.
0x0000555555554ead in main ()

Ok, thanks for the help guys. I implemented some of the changes you guys suggested and it seems to be working fine now.

I think the cause of the error was me not remembering to account for the null terminator when using malloc for inName and outName.

Multiplying by `sizeof (char)` is pointless; `sizeof (char)` is 1 by definition. — melpomene, Feb 11 '18 at 23:07
Please show the _complete_ example. Which header files you #include can make a difference. — Thomas Padron-McCarthy, Feb 11 '18 at 23:08
`strcpy(inName, argv[i+1]);` is a buffer overflow. It tries to write n+1 bytes to a buffer of size n (where n = `strlen(argv[i+1])`). — melpomene, Feb 11 '18 at 23:08
[`while (!feof(in))` is wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong). — melpomene, Feb 11 '18 at 23:09
`in = fopen(inName, "r");` is a resource leak (file handle and memory). It's also missing an error check. — melpomene, Feb 11 '18 at 23:10
`data = malloc((dataSize) * sizeof(int));` is a type error: You want to allocate `dataSize` *pointers*, not `dataSize` *integers*. It should be `dataSize * sizeof (int *)`. — melpomene, Feb 11 '18 at 23:11
`data[i] = malloc(3 * sizeof(int));` is a buffer overflow: In the last iteration of the containing loop, it tries to write to `data[dataSize]`, but the last valid index is `data[dataSize-1]`. The code for `events` has the same bug. — melpomene, Feb 11 '18 at 23:13
Please fix the random indentation in your code. This is hard to read. — melpomene, Feb 11 '18 at 23:15
Run the program under [`valgrind`](http://valgrind.org/) and fix the _first_ problem it reports, repeat until there are no more problems. — zwol, Feb 11 '18 at 23:16
To get better output from gdb and valgrind, compile with the `-g` flag. — melpomene, Feb 11 '18 at 23:16
http://idownvotedbecau.se/toomuchcode/, http://idownvotedbecau.se/nomcve/, http://idownvotedbecau.se/nodebugging/ — Murphy, Feb 11 '18 at 23:43
There is not need to do a `malloc`+`strcpy` of `inName` and `outName`, you are not modifying these strings, etc. `char *inName = NULL;... inName = argv[i+1]` is more than enough here. Besides you are overflowing the memory, because you didn't allocate enough memory (not enough for the 0-terminating byte). — Pablo, Feb 11 '18 at 23:50
You are also overflowing `data`, the `for` loop condition should be `i < datasize;` — Pablo, Feb 11 '18 at 23:54
@Pablo, this is what I thought it should be, however when I get rid of the `+1` then my program crashes. — agentosage, Feb 12 '18 at 00:03
@agentosage the `+1` from where? It should be `malloc(strlen(argv[i+1]) + 1); strcpy(inName, argv[i+1]);` — Pablo, Feb 12 '18 at 00:05
@Pablo I see what you're saying now, yeah I made that change and that seemed to fix my program. I was referring to the `+1` from the `for(int i=0; i < dataSize+1; i++)` statement, which doesn't make sense to me but without the `+1` there my program crashes. — agentosage, Feb 12 '18 at 00:09
@agentosage compile it `-g` and use gdb or valgrind. They'll tell you were things go wrong. — Pablo, Feb 12 '18 at 00:14
@Pablo I am very new to Linux, so sorry for the dumb question, but how do I compile with `-g`?(My program name is just main) I tried `gcc -g main main.c` and it tells me `main error: adding symbols: Bad value` — agentosage, Feb 12 '18 at 00:18
`gcc main.c -o main -g` should do it. The executable file name is `name` which is passed with the `-o` option. — Pablo, Feb 12 '18 at 00:20

Tsakiroglou Fotis · Answer 1 · 2018-02-12T15:40:42.650

You have had various problems. I made some corrections and this code seems to work for me. I just used a valgrind check to find your errors.

In general it's always a good strategy to initialize pointers with FILE *in = NULL;.

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

int main(int argc, char *argv[])
{
    // Declare variables to be used
    FILE *in = NULL;
    FILE *out = NULL;
    char* inName = NULL;
    char* outName = NULL;
    int eventNum, taskNum, daysNum;
    int amtEvents, totalDays = 0;
    int **events;
    int **data, dataSize = 0;
    int currentEvent = 1;

    // Exit program if there are too few arguments entered
    if(argc < 2)
    {
        printf("Too few arguments.");
        return 0;
    }

    // Locate the -input and -output tags to determine
    // file names
    int i;
    for(i=0; i<argc; i++)
    {
        if(! strcmp(argv[i], "-input"))
        {
            inName = malloc(strlen(argv[i+1]) * (sizeof(char)));
            strcpy(inName, argv[i+1]);
        }

        if(!strcmp(argv[i], "-output"))
        {
            outName = malloc(strlen(argv[i+1]) * (sizeof(char)));
            strcpy(outName, argv[i+1]);
        }
    }

    if ((in = fopen(inName, "r")) == NULL){
        fprintf(stderr, "Input file usage: %s", inName);
        return 0;
    }


    out = fopen(outName, "w");

    while (!feof(in)) {
        if (fscanf(in, "%d %d %d", &eventNum, &taskNum, &daysNum) != 3)
            break;
        dataSize++;
    }

    in = fopen(inName, "r");

    // Store the data in the file into a
    // dynamically created 2D array
    data = malloc((dataSize) * sizeof(int));
    while (!feof(in)) {
        for(i=0; i < dataSize+1; i++){
                data[i] = malloc(3 * sizeof(int));

                if (fscanf(in, "%d %d %d", &eventNum, &taskNum, &daysNum) != 3)
                    break;
                data[i][0] = eventNum;
                data[i][1] = taskNum;
                data[i][2] = daysNum;
        }
    }


    // Increment amtEvents for each unique event number
    amtEvents = 1;
    for(i=0; i<dataSize; i++)
    {
        while(currentEvent != data[i][0])
        {
           amtEvents++;
           currentEvent++;
        }
    }

    // Create another 2D array to store data about
    // each event
    events = malloc(amtEvents * sizeof(int));
    for(i=0; i < amtEvents+1; i++)
    {

        events[i] = malloc(3 * sizeof(int));
        int j;
        for(j=0; j<3; j++){
            events[i][j] = 0;
        }

    }


    for(i=0; i < dataSize; i++)
    {

        events[data[i][0]-1][1]++;
        if(data[i][2] > events[data[i][0]-1][2])
            events[data[i][0]-1][2] = data[i][2];
    }
    // Header
    fprintf(out, "Project completion timetable\n");
    fprintf(out, "----------------------------------------\n");
    fprintf(out, "Event\tNum of tasks\tMax num.of days\n");
    fprintf(out, "-----\t------\t\t--------\n");


    for(i=0; i<amtEvents; i++)
    {
        fprintf(out, "%d\t %d\t\t %d\n", i+1, events[i][1], events[i][2]);
        totalDays += events[i][2];
    }

    fprintf(out, "----------------------------------------\n");
    fprintf(out, "Total number of days to finish the project: %d", totalDays);
    free(inName);
    free(outName);
    free(data);
    free(events);

    return 0;
}

**PS: With this code I have a clean bill , in Centos7 - Codeblocks - GNU GCC.

Need help solving Segmentation Fault that only occurs in linux

1 Answers1