1

I have a function that create a linked list of struct.

struct words {
    char * word;
    words next;
};
typedef struct words s_words;
typedef words * Words;

I got a function that create my linked list using this code

Words w = NULL; // start of the list
Words t; // temp node

if (t = (Words) malloc(sizeof(s_words))) {

    t->word = a_string_created;
    t->next = w;
    w = t; // Adding at start of the list
}

If I made a printf("%s",t->word) and printf("%s",a_string_created) I got the same value.

My problem is when I try to retrieve word from an another function.

int readWords(Words list, char * wordRead) {

    if (list != NULL) {
        //strcpy(wordRead,list->word);
        wordRead = list->word;
        return 1;
    }
    else {
        return 0;
    }   
}

I can't get the value in readWords. A printf("%s",list->word) in it give me a strange caracter. And from the caller function 

char rw[11]; //I've try char* rw too
readWords(aList,rw);

printf("%s",rw) print nothing.

I'm stuck with this for hours now. For sure there's something I don't see/understand.

Edit:

I solved partly my problem by replacing t->word = a_string_created; by strcpy(t->word, a_string_created); Now on my printfs, I print string value. But the value have slightly change for some values ex.: test become uest !!

Answer

Change t->word = a_string_created; to t->word = strdup(a_string_created);

Anyone can help and explain to me where and why I'm wrong ?

Improve this question
3
  • BTW: why don't you use the function's return value to return the value, instead of the silly 1 or 0 ? Commented Jul 14, 2013 at 16:51
  • It's a school homework and I can't change the signature for readWords. Commented Jul 14, 2013 at 18:14
  • maybe discuss it with your teacher then. It looks clearly wrong to me. Returning NULL seems to be a good indication of not found ) Commented Jul 14, 2013 at 23:43

4 Answers 4

Reset to default
2

The problem happens because your readWords misuses the buffer: rather than copying the string from the list into it, readWords assigns it as a pointer. Since rw is passed by value, the content of the buffer remains uninitialized, leading to invalid printouts due to undefined behavior.

There are several ways of fixing this problem:

  • Use strcpy to copy the content of list->words into the wordRead buffer, or
  • Change readWords to take a pointer to a pointer for wordRead, and assign it list->words.

The strcpy approach is less safe, unless you take the size of the buffer as well. The pointer to pointer approach looks like this:

int readWords(Words list, char **wordRead) {
    if (list != NULL) {
        *wordRead = list->word;
        return 1;
    } else {
        return 0;
    }
}

The call looks like this:

char *rw;
if (readWords(aList, &rw)) {
    printf("%s", rw)
}
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

It's a school homework and I can't change the signature for readWords. But I can change everything from the "caller function".
@PatrickPellegrino Use strcpy then - you have started along the right track, but then you commented out the right call. If you use strcpy, the assignment would have to go.
I have to comment it because I receive a segmentation fault thinking that was my problem. Maybe my problem is in my List while printf("%s",list->word) in readWords give me bad output.
@PatrickPellegrino You may get a segfault when the word is longer than 10 characters (11 characters when counting the null terminator). Another possibility is that the list element may have been freed, or the pointer inside the list element points to an invalid memory location.
@PatrickPellegrino That's because you did not allocate space to t->word before copying. The pointer points to a random place in memory, causing random overwrites. What you need is either calling strdup (i.e. t->word = strdup(a_string_created); or changing declaration of word from char *word to char word[11].
|
1

You're allocating memory for your struct but failing to allocate memory for your string.

Not only that but your function attempts to set the char * variable wordRead to the beginning of the stored string. But it never returns this value outside the function - which appears to be the intent of the function. Your second function should read:


int readWords(Words list, char **wordRead) {
    if (list != NULL) {
        *wordRead = list->word;
        return 1;
    } else {
        return 0;
    }   
}
Improve this answer

1 Comment

It's a school homework and I can't change the signature for readWords. But I can change everything from the "caller function".
0

The Problem is that you need to pass a double pointer for wordRead. The way you did it, you just modify a local value.

So the correct prototype is int readWords(Words list, char ** wordRead).

Improve this answer

2 Comments

It's a school homework and I can't change the signature for readWords. But I can change everything from the "caller function".
@PatrickPellegrino In that case you have to provide memory by the caller and copy the value to the buffer passed to readWords. - Btw: If the caller passes such a buffer, it's good practice to pass the buffer's size along with the pointer to avoid Overflows.
0

Inside your struct you need to allocate for char*. if there is no space allocated for your char data, your pointer is already pointing to garbage.

Secondly strcpy is not always safe unless you explicitly check the size of data you are trying to copy and make sure you string HAS a '\0' in the end. strncpy is safer than strcpy.

Improve this answer

1 Comment

BTW: strncpy() is **not** safer than strcpy()`. It only has different problems. (such as not null-terminating the result string(

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.