Segmentation fault in C while trying to declare an array of strings.

Question

"fruits.txt" is a text file that starts with a number, say n, followed by the names of n fruits.I want to store those n names into an array of strings, but while trying to declare that array, I am getting "Segmentation fault(core dumped) error.

#include<stdio.h>
#include<stdlib.h>
int main()
{
    FILE *fp;
    int count;
    fp = fopen("fruits.txt", "r");
    if(fp == NULL)
    {
        printf("Can't open file!!");
        exit(0);
    }

    fscanf(fp, "%d", &count);
    printf("%d\n", count);
    char *fruits[count]; // This line is giving Segmentation fault.
    fclose(fp);

    return 0;
}  

Answer 1

Depending on how you wish to provide storage (memory) to hold the fruit names as you read them, you have 2 options (2) use the little know 'm' field modifier (it was 'a' on older implementations so if you are on windoze, read the documentation to make a determination (or just try both and see which one works).

The immediate issue with your code was the fact that the fopen was called with the "w" file mode. That will only work if the fruits.txt already exhisted as the "w" mode will NOT create a file if it doesn't exist. The proper mode is "w+" (or "a" or "a+", either of which will also create a non-existing file). Simply changing your mode to "w+" will allow the fruit info to be written to a newly created fruits.txt.

The issue regarding use of a VLA (variable length array) on some MS compilers may present issues. You will just have to look at your version and the changelogs or documentation (or just try and read the error or warnings. Worse case, you can use a pointer-to-pointer-to-type, or a static array of sufficient size. Given your use of fscanf that approach is continued below. Specifically though:

    char *fruits[count]; /* here you delare an array of pointers to type char   */
                        /* utilizing a variable length array. some MS compiler */
                        /* version do not handle VLA's, VS13/VS15 should be ok */

    for (i = 0; i < count; i++)       /* read & allocate for each array element */
        if (fscanf (fp, " %ms", &fruits[i]) != 1) {    /* m allocates, a for ms */
            fprintf (stderr, "error: read/allocation for fruits[%d] failed.\n", i);
            exit (EXIT_FAILURE);
        }

(Note the format string used above " %ms" which has the caveat that the pointer to accept the allocation block must be a pointer-to-pointer to char * -- which is the address of the pointer to allocate/file, e.g. &fruits[i])

The remainder of the code should be very familiar, except now all input and other critical points in the code have been validated by checking the return provided by whatever function to insure no error condition existed up to that point and that none was caused by the operation in question. That is the only way you can have confidence in the operation of your code. Get in the habit.

Putting that together, you could come up with the following.

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

int main (void) {

    int count, i;
    FILE *fp;

    if (!(fp = fopen ("fruits.txt", "w+"))) {  /* "w+" required to create file */
        fprintf (stderr, "error: file open failed 'fruits.txt'.\n");
        exit (EXIT_FAILURE);
    }
    fputs ("4 Apple Banana mango berry", fp); /* write string to fruits.txt */
    fclose(fp);

    if (!(fp = fopen ("fruits.txt", "r"))) { /* validate file open for reading */
        fprintf (stderr, "error: file open failed 'fruits.txt'.\n");
        exit (EXIT_FAILURE);
    }

    if (fscanf (fp, " %d", &count) != 1) {  /* read all fruir from fruits.txt */
        fprintf (stderr, "error: in read of value from 'fruits.txt'.\n");
        exit (EXIT_FAILURE);
    }
    printf ("\n Quantity read from 'fruits.txt' is '%d'.\n\n", count);

    char *fruits[count]; /* here you delare an array of pointers to type char   */
                        /* utilizing a variable length array. some MS compiler */
                        /* version do not handle VLA's, VS13/VS15 should be ok */

    for (i = 0; i < count; i++)       /* read & allocate for each array element */
        if (fscanf (fp, " %ms", &fruits[i]) != 1) {    /* m allocates, a for ms */
            fprintf (stderr, "error: read/allocation for fruits[%d] failed.\n", i);
            exit (EXIT_FAILURE);
        }
    fclose(fp);  /* close file for the last time */

    for (i = 0; i < count; i++) /* output array */
        printf (" fruit[%d] : %s\n", i, fruits[i]);

    for (i = 0; i < count; i++) /* free allocated memory */
        free (fruits[i]);

    return 0;
}

A basic compile string for the code (in abc.c) and the executable placed in bin/abc could would be:

$ gcc -Wall -Wextra -o bin/abc abc.c -std=gnu11

Example Use/Output

$ ./bin/abc

 Quantity read from 'fruits.txt' is '4'.

 fruit[0] : Apple
 fruit[1] : Banana
 fruit[2] : mango
 fruit[3] : berry

In any code your 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 insure you haven't written beyond/outside your allocated block of memory, attempted to read or base a jump on an unintitialized value and finally to confirm that you have freed all the memory you have allocated. For Linux valgrind is the normal choice.

$ valgrind ./bin/abc
==30980== Memcheck, a memory error detector
==30980== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==30980== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==30980== Command: ./bin/abc
==30980==

 Quantity read from 'fruits.txt' is '4'.

 fruit[0] : Apple
 fruit[1] : Banana
 fruit[2] : mango
 fruit[3] : berry
==30980==
==30980== HEAP SUMMARY:
==30980==     in use at exit: 0 bytes in 0 blocks
==30980==   total heap usage: 10 allocs, 10 frees, 1,561 bytes allocated
==30980==
==30980== All heap blocks were freed -- no leaks are possible
==30980==
==30980== For counts of detected and suppressed errors, rerun with: -v
==30980== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1)

Always confirm All heap blocks were freed -- no leaks are possible and equally important ERROR SUMMARY: 0 errors from 0 contexts. (although note: some OS's do not provide adequate memeory exclusion files (the file that excludes system and OS memory from being reported as in use) which will cause valgrind to report that some memory has not yet been freed (despite the fact you have done your job and freed al blocks you allocoted and under your control.

That's a lot to take in for a simple problem, and we didn't even talk about the preferred manner of reading the file would have been to read and entire line at a time with fgets or POSIX getline and then to either parse the individual fruits from the line read, or to tokenize the line with strtok. Take the time to digest the cocde and answer for yourself the two compiler questions you must look up (1) support of VLA's for fruits[count] and (2) whether m or a is use by your compiler to allocate.

Answer 2

I've tried below code and found working. The difference here is first i've created the file and filled the content in it.

If i don't fill contents in file then i gets segmentation fault (not on all compilers). So it seems in your case the fscanf() is reading some junk and returning some big junk number. As others also suggested, please check what fscanf returns. if it returns-1, means nothing is found in your file.

#include<stdio.h>
#include<stdlib.h>
int main()
{
    FILE *fp;
    int count;
    int i;

    fp = fopen ("fruits.txt", "w");
    if(fp == NULL)
    {
        printf("Can't open file!!");
        exit(0);
    }
    fputs("4 Apple Banana mango berry", fp);
    fclose(fp);*/

    fp = fopen("fruits.txt", "r");
    if(fp == NULL)
    {
        printf("Can't open file!!");
        exit(0);
    }

    fscanf(fp, "%d", &count);
    printf("%d\n", count);

    fclose(fp);
    char *fruits[count]; // This line is giving Segmentation fault.

    return 0;
} 

This prints 4 with no error/fault.

Answer 3

When using fscanf, you must always check the return value. Only if the function returns a successful value (read the fscanf documentation for that), the program should continue.