CODEWITHSUNDEEP
carraysstruct
user4593274
user4593274

Reputation:

Cant Add Item to Array of Structs in C

I cant add an item to array of struct and couldnt figure out why. This is how it looks like: Struct:

typedef struct node
{
    char *data;
    int count;
};

Array initialization:

struct node* list = malloc(100 * sizeof(struct node));

Add Part: (buffer is read from file)

fscanf( fp, "%s", &buffer);

list[ index].data = (char*)malloc(strlen( buffer));
strcpy( list[ index].data, buffer);
list[ index].count = 0;
index++;

Upvotes: 0

Views: 137

Answers (1)

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53006

There are three problems with your code

  1. It's unsafe, using fscanf() like that is dangerous. You need to tell fscanf() to stop reading after a certain ammount of characters to avoid buffer overflow, so for example

    char buffer[100];
if (fscanf(fp, "%99s", buffer) != 1)
    doNot_Try_to_copy_buffer_itWasNotInitialized();

  2. You are passing the address of buffer to fscanf() which is wrong, why? depends on how you declared buffer, if you declared it like

    char buffer;

    it's wrong because you willll have space for just one character and almost surely your program will invoke undefined behavior, if you declared it as

    char buffer[SOME_REASONABLE_SIZE];

    then the problem is that sizeof(buffer[0]) != sizeof(&buffer[0]) and hence pointer arithmetic will be a problem inside fscanf().

  3. It's wrong, you are allocating the wrong ammount for the data field. A c string consists of a sequence of non-nul bytes followed by a nul byte, you are allocating space for the non-nul part only since strlen() returns the number of non--null characters, strcpy() will copy the '\0' and hence your program will invoke undefined behavior, the correct way of duplicating a string is

    size_t length = strlen(buffer);
list[index].data = malloc(1 + length);
if (list[index].data != NULL)
    memcpy(list[index].data, buffer, 1 + length);

    note that i've used memcpy() because the length was alreadly computed with strlen() so I don't want strcpy() searching for the '\0' again.

Your code is unsafe because you ignore the return value of the functions you use, which can lead to undefined behavior.

Note: As mentioned in comments, casting void * in c is discouraged and unnecessary, and most c programmers don't appreciate that for the issues mentioned in the link posted by @SouravGhosh.

Upvotes: 1

Related Questions