[SOLVED] C

Mechanikx · 04-27-2020, 08:12 PM

I'm writing a simple program to read in a file of one word per line and store that word into a node of a linked list then print. For simplicity, I removed error checking, etc. Here is the program:

Code:

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

struct node {
    char *str;
    struct node *next;
};

int
main(void)
{
    struct node *current = NULL, *head = NULL, *prev = NULL;
    char buffer[256];
    char read_buffer[256];
    FILE *fp = NULL;
    size_t bufsz;

    fp = fopen("words.txt", "r");

    while ((fgets(read_buffer, sizeof(read_buffer), fp)) != NULL)
    {
        bufsz = strlen(read_buffer);
        current = malloc(sizeof(struct node));
        current->str = malloc(bufsz);
        strncpy(current->str, read_buffer, bufsz);
        current->next = NULL;
        
        if (head == NULL)
            head = current;
        else
            prev->next = current;

        prev = current;
    }

    fclose(fp);
    current = head;

    while (current != NULL)
    {
        printf("%s", current->str);
        current = current->next;
    }

    current = head;

    while (current != NULL)
    {
        head = current->next;
        free(current->str);
        free(current);
        current = head;
    }

    return 0;
}

Here is the file it reads (words.txt):

Quote:

Red
Orange
Yellow
Green
Blue
Indigo
Violet

Which works as expected.

However, I originally had a small bug. For this line:

Quote:

strncpy(current->str, read_buffer, bufsz);

I originally wrote:

Quote:

strncpy(current->str, buffer, bufsz);

So, although each word was being transferred from the file into read_buffer, read_buffer wasn't being transferred to the memory pointed to by current->str.

When I run the buggy version all that happens is I get back a prompt but I expected garbage values to be printed since buffer is being used uninitialized.

I created a smaller version of this program (only creates one node and doesn't read from a file) to see if my expectation was correct and it was:

Code:

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

struct node {
    char *str;
    struct node *next;
};

int
main(void)
{
    size_t bufsz;
    struct node *current = NULL;
    char buffer[256];
    char read_buffer[256] = "Red\n";

    bufsz = strlen(read_buffer);
    current = malloc(sizeof(struct node));
    current->str = malloc(bufsz);
    current->next = NULL;

    strncpy(current->str, buffer, bufsz);
    printf("%s\n", current->str);

    free(current->str);
    free(current);

    return 0;
}

which prints a garbage value:

Quote:

.N=ö

Why isn't the original buggy version printing garbage values? I feel like I could be missing something quite obvious but I can't figure it out. Any help would be greatly appreciated.

rtmistler · 04-27-2020, 08:39 PM

Why don't you examine the contents of buffer in your original version?

Either add some debug, and dump it out in hex, or use gdb and examine the memory.

The debugger also will let you step through and examine each loop iteration and give you very clear explanations.

EdGr · 04-27-2020, 09:27 PM

C strings are terminated by '\0'. buffer[0] is likely coming up as '\0' which is the empty string.
Ed

Mechanikx · 04-27-2020, 10:18 PM

I ran both programs through GDB and printed the contents of the buffer in the first program which started with:

Code:

\003

Which is the octal value for 'end of text'. I can't remember the value for the second program but it wasn't \003 or \000.

Running the first program through GDB a second time and printing the contents of buffer it then started with \000, the value for the Null character.

I added this statement:

Code:

printf("d\n", buffer[0]);

After the buffer declaration in each program in my OP.

For the first one I got:

Quote:

0

and for the second one I got:

Quote:

46
.N=ö

So in the first instance I'm getting the decimal value for the Null or for the 'end of text' character and in the second instance I'm not which explains the results I was getting.

Why does this only seem to happen with the first program and not the second? Are the programs using the same region of memory over and over again? If I ran these programs enough times would the results change? I supposed I could write a script to test this out.

EdGr · 04-27-2020, 10:26 PM

Quote:

Originally Posted by Mechanikx

Why does this only seem to happen with the first program and not the second? Are the programs using the same region of memory over and over again? If I ran these programs enough times would the results change? I supposed I could write a script to test this out.

Since buffer is uninitialized, every time you change the program, or change how it is run (with/without the debugger), the uninitialized value may change.

The moral is: never read an uninitialized variable. gcc is very good at detecting uninitialized variables (compile with -Wall).
Ed

Mechanikx · 04-27-2020, 10:39 PM

Quote:

Originally Posted by EdGr

Since buffer is uninitialized, every time you change the program, or change how it is run (with/without the debugger), the uninitialized value may change.

The moral is: never read an uninitialized variable. gcc is very good at detecting uninitialized variables (compile with -Wall).
Ed

Much appreciated. I always compile with -Wall and buffer is used in another part of the program that I omitted. It was just a mistake on my part that I read from it. I thought buffer's contents, not being initialized, would've changed more regularly and buffer in the first program wouldn't have started with either /000 or /003 as consistently as it did, so I thought comparing those two programs like I did was enough. I'll never make such assumptions again.

bigearsbilly · 05-01-2020, 05:21 AM

Don't waste your time looking at bad code, look at _good_ code instead.

These problems are like trying to analyse:

I jump off a roof and sometimes I die, sometimes I break a leg.

syg00 · 05-01-2020, 06:16 AM

Just proves Uninitialised data ain't. Unless it's zeroes, in which case it also ain't.

rtmistler · 05-01-2020, 07:53 AM

-Wall is not all warning flags.

If really interested, check gcc's manpage and search for -W and also read which flags -Wall does include.

There seem to be some new tendencies where compilers do initialize variables to zero. There are compile switches, however on someone else's question, the same uninialized condition occurred for them, but I checked on my system and found with normal compilation and no added flags, it was clearing local variables. Either case, I recommend initializing variables before using them.

SoftSprocket · 05-01-2020, 08:18 AM

Quote:

Originally Posted by rtmistler

-Wall is not all warning flags.

If really interested, check gcc's manpage and search for -W and also read which flags -Wall does include.

There seem to be some new tendencies where compilers do initialize variables to zero. There are compile switches, however on someone else's question, the same uninialized condition occurred for them, but I checked on my system and found with normal compilation and no added flags, it was clearing local variables. Either case, I recommend initializing variables before using them.

Yes, I'm reasonably sure there is nothing in the C specification where variables are initialized to anything (and probably nothing that says they can't be) so it would be a mistake to have an expectation of a known value in uninitialized variables.

NevemTeve · 05-01-2020, 11:47 AM

Well, 'uninitialized' doesn't mean 'source of random data'. Your program runs in separate virtual memory, where memory pages are zero-initialized by the kernel (for security reasons: you're not supposed to see any data the previous program might have left there). So if your automatic variable (that exists on the stack) contains non-zero value, then it came from your own program.

GazL · 05-01-2020, 05:26 PM

Quote:

Originally Posted by SoftSprocket

Yes, I'm reasonably sure there is nothing in the C specification where variables are initialized to anything (and probably nothing that says they can't be) so it would be a mistake to have an expectation of a known value in uninitialized variables.

This seems to be the salient part of the C11 standard: section 6.7.9-10

Quote:

If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate. If an object that has static or thread storage duration is not initialized explicitly, then:
— if it has pointer type, it is initialized to a null pointer;
— if it has arithmetic type, it is initialized to (positive or unsigned) zero;
...

So, short answer: if you want to be lazy, define it as static.

You probably don't want to be putting large buffers on the stack anyway, so unless you have some thread-safety issues to worry about, or you're doing some recursion, static might not be such a bad idea.

@Mechanikx, if I may make a suggestion: instead of malloc() and strncpy()

Code:

 
current->str = strdup(read_buffer);