0

I am creating a function to load a Hash Table and I'm getting a segmentation fault if my code looks like this

bool load(const char *dictionary)
{
    // initialize vars
    char *line = NULL;
    size_t len = 0;
    unsigned int hashed;

    //open file and check it
    FILE *fp = fopen(dictionary, "r");
    if (fp == NULL)
    {
        return false;
    }

    while (fscanf(fp, "%s", line) != EOF)
    {
        //create node
        node *data = malloc(sizeof(node));
        //clear memory if things go south
        if (data == NULL)
        {
            fclose(fp);
            unload();
            return false;
        }

        //put data in node
        //data->word = *line;
        strcpy(data->word, line);

        hashed = hash(line);
        hashed = hashed % N;

        data->next = table[hashed];
        table[hashed] = data;
        dictionary_size++;
    }

    fclose(fp);
    return true;
}

However If I replace

char *line = NULL; by char line[LENGTH + 1]; (where length is 45)

It works. What is going on aren't they "equivalent"?

MiguelP
  • 416
  • 2
  • 12
  • 3
    The latter allocates memory; the former does not. "char *line" says nothing more than "I want a variable named 'line' that can point to characters", but it doesn't actually point to any. – Lee Daniel Crocker Jul 30 '21 at 17:28
  • 1
    Who said they were equivalent? Clearly `char line[LENGTH + 1]` is talking about some number of bytes -- specifically `LENGTH + 1` of them. But just about equally clearly `char *line = NULL` involves a pointer which points definitely nowhere, certainly not to any well-defined number of properly allocated bytes! – Steve Summit Jul 30 '21 at 17:45
  • `char line[LENGTH + 1];` means `line` is an array, `char *line = NULL;` means `line` is a pointer. This are completely different things but array variables often decay to pointers when you use. – 12431234123412341234123 Jul 30 '21 at 20:03
  • See also this question https://stackoverflow.com/questions/1641957/is-an-array-name-a-pointer – 12431234123412341234123 Jul 30 '21 at 20:06

4 Answers4

5

When you do fscanf(fp, "%s", line) it'll try to read data into the memory pointed to by line - but char *line = NULL; does not allocate any memory.

When you do char line[LENGTH + 1]; you allocate an array of LENGTH + 1 chars.

Note that if a word in the file is longer than LENGTH your program will write out of bounds. Always use bounds checking operations.

Example:

while (fscanf(fp, "%*s", LENGTH, line) != EOF)
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
3

They are not equivalent.

In the first case char *line = NULL; you have a pointer-to-char which is initialised to NULL. When you call fscanf() it tries to write data to it and this will cause it to dereference the NULL pointer. Hence segfault.

One option to fix that would have been to allocate (malloc() and friends) the required memory first, check the pointer is not NULL (allocation failed) before using it. Then you would need to free() the resources once you no longer need the data.

In the second case char line[LENGTH +1] you have an array-of-char of size LENGTH + 1. This memory has been allocated for you on the stack (the compiler ensures this happens automatically for arrays), and the memory is only 'valid' for use during the lifetime of the function: once you return you must no longer use it. Now, when you pass the pointer to fscanf() (to the first element of the array in this case), fscanf() has a memory buffer to write to. As long as the buffer is large enough to hold the data being written this works correctly.

user268396
  • 11,576
  • 2
  • 31
  • 26
2
char *line = NULL;

Says "I want a variable named 'line' that can point to characters, but is not currently pointing to anything." The compiler will allocate memory that can hold a memory address, and will fill it with zero (or some other internal representation of "points to nothing").

char line[10];

Says "allocate memory for 10 characters, and I would like to use the name 'line' for the address of the first one". It does not allocate space to hold the memory address, because that's a constant, but it does allocate space for the characters (and does not initialize them).

Lee Daniel Crocker
  • 12,927
  • 3
  • 29
  • 55
0

Declaring a pointer as NULL doesn't allocate memory for the array. When you access the pointer, then what gets executed is reading / writing to a null pointer, which is not what you want. How fscanf works is it writes out to the buffer you sent, hence meaning that the buffer must be allocated before hand. If you want to use a pointer, then you ought to do:

    char* line = malloc(LEN + 1);

When declaring as an array, then the compiler allocates memory for it, not you. This is better, in case you forget to free the memory, which the compiler won't do. Note that if you do use an array (which is a local variable in this case), it cannot be used by functions higher up on the call stack, because as I stated above, the memory gets freed upon return from the function.

nexos
  • 66
  • 3