Cant Add Item to Array of Structs in C

Question

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++;

Standard Warning : Please [do not cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family. — Sourav Ghosh, Mar 10 '15 at 10:32
1) what makes you feel you `can't add`? 2) did you initialize `index`? 3) always check the return value of `fscanf()`. 4) instead of `malloc()` and `strcpy()`, you can use `strdup()` directly. — Sourav Ghosh, Mar 10 '15 at 10:32
Note that you don't allocate enough space for data. `strlen` doesn't include nul character, so you should add `+1`. — user694733, Mar 10 '15 at 10:35

score 1 · Accepted Answer · edited May 23 '17 at 12:22

There are three problems with your code

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();
```
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().
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.

@user694733 I will add as much detail as possible. Is it Ok now? — Iharob Al Asimi, Mar 10 '15 at 10:41

Cant Add Item to Array of Structs in C

1 Answers1