Cannot read in all names from text file in C

Question

I am writing a program that reads in names from a file. I am having trouble reading in all the names without it Segmentation Faulting. Here is my code.

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

typedef struct node{
        char *name;
        int index;
}node;

int main(void){
        char *someString;
        struct node *TEST;

        TEST = (node*)malloc(sizeof(node));

        FILE *filePointer;
        filePointer = fopen("data2.txt","r");

        int lineCount = 0;
        //while(fscanf(filePointer, "%s", &someString) != EOF){
        while(1){
                if (!feof(filePointer)){
                        fscanf(filePointer, "%s", &someString);
                        printf("String from file: %s\n", &someString);                 
                }
                else{
                        break;         
                }
        }
}

`char **` (which is what `&someString` yields) is not the correct type for reading a `"%s"`. A `char *` with sufficient space addressed by said-same, would be the approach to take. The loop logic is effectively `while (!feof(filePointer))`, [which is also wrong](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong?s=2|2.9699). — WhozCraig, May 11 '15 at 03:11
Tried that but it goes into an infinite loop. Scanning nothing — user1881401, May 11 '15 at 03:19
'someString' is not initialized. TEST is, but is left unused. You would learn about if you compiled this code with warnings enabled. camelCase in C code is a criminal offense. — , May 11 '15 at 03:22
Put another way: `&someString` is *wrong* in *both* places it is used in this code. That is the short of it. Your code is executing *undefined behavior* if you read any number of chars greater than one-byte-less than the size of a pointer (which is likely 4 on a 32bit platform, 8 on a 64bit platform). You need a char buffer for reading with [**`fscanf`**](http://en.cppreference.com/w/c/io/fscanf) using `%s` , or writing using [**`printf`**](http://en.cppreference.com/w/c/io/fprintf). You have none. All you have is an indeterminate pointer. — WhozCraig, May 11 '15 at 03:27

score 1 · Answer 1 · answered May 11 '15 at 04:15

Your use of &somePointer in both locations in this code is wrong. The result of &somePointer is char**. When applying the unary address-of operator &, the result is pointer-to-type. Since somePointer is a char*, that means &somePointer is a char**, which is not what you want. It is wrong usage of the %s format specifier using scanf and likewise for printf.

Both of those functions require an address of a char buffer, not the address of a char pointer. While you may think just using somePointer will work, in fact it will not, as even then that variable is indeterminate. You never provide the pointer with a valid address where a char buffer resides.

And, your while-loop logic is wrong. it is equivalent to a while(!feof(filePointer)) loop, which is nearly-always wrong. You can read more about it here

The following is a simple slab of code that safely reads strings up to 127 chars wide from a file until the file is exhausted or an error is encountered:

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

int main()
{
    char buff[128];

    FILE *fp = fopen("data2.txt","r");
    if (fp == NULL)
    {
        perror("data2.txt");
        return EXIT_FAILURE;
    }

    while (fscanf(fp, "%127s", buff) == 1)
    {
        printf("String from file: %s\n", buff);
    }

    fclose(fp);

    return 0;
}

Adding a pointer to this gives the slightly modified:

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

int main()
{
    char buff[128];
    char *somePointer = buff;

    FILE *fp = fopen("data2.txt","r");
    if (fp == NULL)
    {
        perror("data2.txt");
        return EXIT_FAILURE;
    }

    while (fscanf(fp, "%127s", somePointer) == 1)
    {
        printf("String from file: %s\n", somePointer);
    }

    fclose(fp);

    return 0;
}

Next, dynamic allocation instead of a fixed automatic buffer. That gives us:

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

int main()
{
    char *somePointer = malloc(128);
    if (somePointer == NULL)
    {
        perror("Failed to allocate buffer space");
        return EXIT_FAILURE;
    }

    FILE *fp = fopen("data2.txt","r");
    if (fp == NULL)
    {
        perror("data2.txt");
        return EXIT_FAILURE;
    }

    while (fscanf(fp, "%127s", somePointer) == 1)
    {
        printf("String from file: %s\n", somePointer);
    }

    fclose(fp);

    free(somePointer);

    return 0;
}

In each case above, the argument passed to fscanf and printf is ultimately a pointer-to-char. the first is done by conversion, the latter two explicitly. Which of these applies to your usage needs I leave to you.

The rest I leave to you. Best of luck!

score 0 · Answer 2 · answered May 11 '15 at 03:28

You are seg faulting because you never actually allocate a place to store the string. "someString" and node->name are both character pointers, but what do they point to? When you run your malloc, you are allocating a structure which contains a pointer, but again, what does it point to?

What you actually need is to declare a place to put the string, like this:

    #define MAX_STRING_SIZE 50
    char thisIsTheActualString[MAX_STRING_SIZE];
    char *someString = thisIsTheActualString;

That last line (where we assign to someString) is completely redundant, I'm leaving it in to show the relationship between the pointer you declared and the actual allocated memory holding the string.

If you don't do this, then when you get to the fscanf statement in your while loop, you instruct the compiler to store the data at the address contained in someString... which we never actually assigned to anyything. As a result, you're writing off to some random memory address, which is where you get the segfault.

Cannot read in all names from text file in C

2 Answers2