CODEWITHSUNDEEP
cpointersdata-structures
mohamad zbib
mohamad zbib

Reputation: 75

valgrind Invalid write of size 8 in a allocated memory

I am trying to add new node to my linked list, but it's gives memory error

my struct and global vars:

typedef struct word word;

struct word
{
    char str[256];
    word *next;
};

word *head = NULL;
word *cur = NULL;

the function :

int addWord(char * str)
{
    word *w = calloc(1, sizeof(w));

    if(w == NULL)
    {
        return 0;
    }  
    strcpy(w->str, str);

    if(cur == NULL)
    {
        cur = w;
        head = w;
    }
    else
    {
puts("4");
        cur->next = w;
puts("5");
        cur = w;
puts("6");    
    }
    return 1;
}

and the result is :

...
4
5
6
4
==73913== Invalid write of size 8
==73913==    at 0x109425: addWord (in /home/mz37/programming/godaphy/bin/godaphy.out)
==73913==    by 0x109696: parseLine (in /home/mz37/programming/godaphy/bin/godaphy.out)
==73913==    by 0x109351: main (in /home/mz37/programming/godaphy/bin/godaphy.out)
==73913==  Address 0x4a6a880 is 96 bytes inside an unallocated block of size 4,188,096 in arena "client"
==73913== 
5
6

i am still searching for the error and i haven't found it yet

Upvotes: 0

Views: 465

Answers (1)

paxdiablo
paxdiablo

Reputation: 881423

word *w = calloc(1, sizeof(w));

The w variable is of type word pointer hence is likely to be four or eight bytes at most. It may be larger if we end up with 128-bit machines at some point, but it'll be quite some time before it gets to 2000+ bits :-)

You probably wanted to do:

word *w = calloc(1, sizeof(*w));
//            note this ___^

The type of *w is the actual type word, and that will be the correct size for what you're trying to do.

And, as an aside, you may want to think about the wisdom of blindly copying whatever string you're given, into a block of memory that can only hold 256 characters. A safer alternative would be:

strncpy(w->str, str, sizeof(w->str) - 1);
// Would normally also do something like:
//     w->str[sizeof(w->str) - 1] = '\0';
// but calloc() makes that superfluous.

The resultant function (including compactifying) would be along the following lines:

int addWord(char *str) {
    word *w;

    // Option to fail if string too big, rather than truncate.

    //if (strlen(str) >= sizeof(w->str)
    //    return 0;

    // Allocate and check.

    if ((w = calloc(1, sizeof(*w)))  == NULL)
        return 0;

    // Copy in string, truncate if too big.

    strncpy(w->str, str, sizeof(w->str) - 1);

    // Make new list if currently empty, otherwise add it, then flag success.

    if(cur == NULL) {
        cur = head = w;
    } else {
        cur->next = w;
        cur = w;
    }

    return 1;
}

Upvotes: 1

Related Questions