C: Segfault caused by assigning string to struct array using strcpy

Question

I am reading in a text file containing 10 rows and 3 columns. Each element needs to be put into an array for further manipulation. The fileToArray method will single out each string and i can successfully print each string individually however I cannot seem to assign the strings to each index correctly.

My understanding is that i have a pointer to 10 dataArrays and in my switch statement what essentially happens is dataArray[0].pName = P1 ... dataArray[1].pName = P2 ... etc... and similarly for the other variables in the struct.

The text file has a max of 10 rows hence why 10 arrays are initialized.

Is my understanding flawed or is there some blatant errors in my code?

struct Processes
{
    char *pName;
    char *arvTime;
    char *srvTime;
};

void fileToArray(FILE *fp, struct Processes dataArray[])
{
    // temp[14] because 14 is the max size a line can be
    char temp[14];
    char delim[] = " \n";
    int count = 0;
    int a, b, c = 0;

    while(fgets(temp, 14, fp) != NULL)
    {
        char *ptr = strtok(temp, delim);

        while(ptr != NULL)
        {
            // printf("'%s'\n", ptr);
            // ^^^^ This line will successfully print out each string

            switch(count)
            {
            case 0:
                dataArray[a].pName = malloc(strlen(ptr + 1));
                strcpy(dataArray[a].pName, ptr);
                a++;
                count++;
                free(dataArray[a].pName);
                break;

            case 1:
                dataArray[b].arvTime = malloc(strlen(ptr + 1));
                strcpy(dataArray[b].arvTime, ptr);
                b++;
                count++;
                free(dataArray[b].arvTime);
                break;

            case 2:
                dataArray[c].srvTime = malloc(strlen(ptr + 1));
                strcpy(dataArray[c].srvTime, ptr);
                c++;
                count = 0;
                free(dataArray[c].srvTime);
                break;
            }


            ptr = strtok(NULL, delim);

        }
    }
}

int main(int argc, void *argv[])
{
    struct Processes dataArray[10];

    if(argc == 1)
    {
        FILE *fp;

        fp = fopen("process-data.txt", "r");

        fileToArray(fp, dataArray);

        fclose(fp);
    }
    return 0;
}

// Sample text file being read //
P1 1 5
P2 2 2
P3 11 5
P4 17 9
P5 3 1
P6 10 10
P7 4 3
P8 4 1
P9 7 8
P10 5 4

Running this code results in a segfault and i am unsure as to why. Looking at other similar posts the solution seems to be not using malloc() which makes me think i have implemented the function incorrectly.

Here dataArray[a].pName = malloc(strlen(ptr + 1)); there is used uninitialized variable a. So the program already has undefined behavior. — Vlad from Moscow
– Vlad from Moscow, Commented Oct 4, 2019 at 13:58
This won't fix your problem, but int main(int argc, void *argv[]) isn't a valid definition of main -- your choices are int main(void) or int main(int argc char *argv[])/char** argv — Reticulated Spline
– Reticulated Spline, Commented Oct 4, 2019 at 14:05
Why do you free the data after having copied the strings to it? Now there is no storage allocated to it anymore. Trying to access it (which you don't show) will cause the seg fault. — Paul Ogilvie
– Paul Ogilvie, Commented Oct 4, 2019 at 14:07
Using three indexes, a, b and c, seems a bit useless. You want each array entry filled with the three data parts. One index should be enough. — Paul Ogilvie
– Paul Ogilvie, Commented Oct 4, 2019 at 14:09
Be aware that int a, b, c = 0; only initializes c to 0. a and b are not initialized, IOW they will initially contain an indeterminate value. You probably want this: int a = 0, b = 0, c = 0; — Jabberwocky
– Jabberwocky, Commented Oct 4, 2019 at 14:28

Chris Turner · Accepted Answer · 2019-10-04 14:31:24Z

2

This bit of code is doing the exact opposite of what you think

malloc(strlen(ptr + 1))

Instead of allocating enough space for the string plus 1 extra for the NUL at the end, it is allocating one less than the length of the string. You want to move the + 1 to the outside of call to strlen like this

malloc(strlen(ptr)+1)

Also you should initialise the values of a and b to 0 or better yet, combine a, b and c into one variable and only increment it when you've read in the 3rd piece of information.

answered Oct 4, 2019 at 14:31

Chris Turner

8,1421 gold badge16 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Vlad from Moscow · Accepted Answer · 2019-10-04 14:53:26Z

For starters you should to pass to the function the number of elements in the array. So the function declaration should look like

void fileToArray( struct Processes dataArray[], size_t n, FILE *fp );

And it is better when the function returns the number of the filled elements of the array.

size_t fileToArray( struct Processes dataArray[], size_t n, FILE *fp );

and the function can be called at least like

size_t n = fileToArray( dataArray, 10, fp );

In this declaration

int a, b, c = 0;

only the variable c is initialized. All other variables, a, and b, are not initialized. So for example this statement

dataArray[a].pName = malloc(strlen(ptr + 1));

results in undefined behavior.

This expression

malloc(strlen(ptr + 1))

is invalid. It is equivalent to

malloc(strlen( &ptr[1] ))

And there is no any sense to free memory at once after its allocation.

free(dataArray[a].pName);

The function could be defined the following way

size_t fileToArray( struct Processes dataArray[], size_t n, FILE *fp )
{
    // temp[14] because 14 is the max size a line can be
    char temp[14];
    char delim[] = " \n";

    size_t i = 0;

    for( ; i < n && fgets( temp, sizeof( temp ), fp) != NULL; i++ )
    {
        dataArray[i].pName   = NULL;
        dataArray[i].arvTime = NULL;
        dataArray[i].srvTime = NULL;

        char *ptr = strtok( temp, delim );

        for( size_t j = 0; j < 3 && ptr != NULL; j++ )
        {
            switch ( j )
            {
            case 0:
                dataArray[i].pName = malloc( strlen( ptr ) + 1 );
                strcpy( dataArray[i].pName, ptr );
                break;

            case 1:
                dataArray[i].arvTime = malloc( strlen( ptr ) + 1 );
                strcpy( dataArray[i].arvTime, ptr );
                break;

            case 2:
                dataArray[i].srvTime = malloc( strlen( ptr ) + 1 );
                strcpy( dataArray[i].srvTime, ptr );
                break;
            }

            ptr = strtok( NULL, delim );
        }
    }

    return i;
}

Collectives™ on Stack Overflow

C: Segfault caused by assigning string to struct array using strcpy

2 Answers 2

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related