C not storing first entry in array correctly

Question

I'm trying write a program to read in a text file and put its contents into an array.This way, you can read any file and regardless of the string length, it will build an array dynamically and fill it in with a file. I'm using this as an exercise to practice with C and hopefully extrapolate this to other types and structs.

However, for some reason, my first entry does not match resulting in unexpected behavior. I understand that with C, you need to essentially micro manage all of your memory, and working with the code, I tried to allocate memory for each entry, but is this the correct approach? I ran the code through my head, and it makes sense logically when starting with 0 entries, but I don't understand why the first entry fails while the remaining entries work.

Code:

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

int main(int argc, char *argv[]){

    //Initialize variables and pointers
    //Create an array of chars to use when reading in
    //Create an array of strings to store
    //i : use to keep track of the number of strings in array
    //j : loop variable
    //size: size of string
    char *s = (char *) malloc(sizeof(char));
    int i=0,j=0;
    int size = 0;
    char **a = (char **) malloc(sizeof(char *));

    //Read in string, assign string to an address at a[]
    while( scanf("%79s",s) == 1){

        //Get the size of the input string
        size = (unsigned) strlen(s);

        //Print some notes here
        printf("\nString is \"%-14s\"\tSize is %-3d, i is currently %d\n",s,size,i);
        printf("Using MALLOC with %d bytes\n",size+1);

        //Allocate memory to copy string
        //
        //For some reason, the commented code works
        //a[i] = (char *) (malloc(sizeof(char)*(size+1)) + 'a');
        a[i] = (char *) (malloc(sizeof(char)*(size+1)) );

        //Go and allocate memory for each character in string to store
        for(j=0; j<(size+1); j++)   a[i][j] = (char) (malloc(sizeof(char)));

        //Print some more notes here
        printf("Size: a[%2d] is %3d bytes, *a[%2d] is %3d bytes, Length of a[%2d] is %d\n",i,(int) sizeof(a[i]),i,(int) sizeof(*a[i]),i,(unsigned) strlen(a[i]));

        //Copy over string and set last char to be end
        for(j=0; j<size; j++)       a[i][j] = (char) s[j];
        a[i][size] = '\0';

        //Print it out and increase i
        printf("a[%3d] is now %s\n",i,a[i]);

        i++;
    }
    printf("I is now %d\n\n\n",i);
    a[i] = NULL;

    //print out array
    for(j=0; j<i; j++)      printf("%3d. %-40s\n",j,a[j]);


    return 0;
}

Test Text File (numbers.txt):

1
22
333
4444
55555
666666
7777777
88888888
9999999
0000000000
11111111111
222222222

Command:

./a.out < numbers.txt

Results:

String is "1             "      Size is 1  , i is currently 0
Using MALLOC with 2 bytes
Size: a[ 0] is   8 bytes, *a[ 0] is   1 bytes, Length of a[ 0] is 2
a[  0] is now 1

String is "22            "      Size is 2  , i is currently 1
Using MALLOC with 3 bytes
Size: a[ 1] is   8 bytes, *a[ 1] is   1 bytes, Length of a[ 1] is 3
a[  1] is now 22

String is "333           "      Size is 3  , i is currently 2
Using MALLOC with 4 bytes
Size: a[ 2] is   8 bytes, *a[ 2] is   1 bytes, Length of a[ 2] is 4
a[  2] is now 333

String is "4444          "      Size is 4  , i is currently 3
Using MALLOC with 5 bytes
Size: a[ 3] is   8 bytes, *a[ 3] is   1 bytes, Length of a[ 3] is 5
a[  3] is now 4444

String is "55555         "      Size is 5  , i is currently 4
Using MALLOC with 6 bytes
Size: a[ 4] is   8 bytes, *a[ 4] is   1 bytes, Length of a[ 4] is 6
a[  4] is now 55555

String is "666666        "      Size is 6  , i is currently 5
Using MALLOC with 7 bytes
Size: a[ 5] is   8 bytes, *a[ 5] is   1 bytes, Length of a[ 5] is 7
a[  5] is now 666666

String is "7777777       "      Size is 7  , i is currently 6
Using MALLOC with 8 bytes
Size: a[ 6] is   8 bytes, *a[ 6] is   1 bytes, Length of a[ 6] is 8
a[  6] is now 7777777

String is "88888888      "      Size is 8  , i is currently 7
Using MALLOC with 9 bytes
Size: a[ 7] is   8 bytes, *a[ 7] is   1 bytes, Length of a[ 7] is 9
a[  7] is now 88888888

String is "9999999       "      Size is 7  , i is currently 8
Using MALLOC with 8 bytes
Size: a[ 8] is   8 bytes, *a[ 8] is   1 bytes, Length of a[ 8] is 8
a[  8] is now 9999999

String is "0000000000    "      Size is 10 , i is currently 9
Using MALLOC with 11 bytes
Size: a[ 9] is   8 bytes, *a[ 9] is   1 bytes, Length of a[ 9] is 11
a[  9] is now 0000000000

String is "11111111111   "      Size is 11 , i is currently 10
Using MALLOC with 12 bytes
Size: a[10] is   8 bytes, *a[10] is   1 bytes, Length of a[10] is 12
a[ 10] is now 11111111111

String is "222222222     "      Size is 9  , i is currently 11
Using MALLOC with 10 bytes
Size: a[11] is   8 bytes, *a[11] is   1 bytes, Length of a[11] is 10
a[ 11] is now 222222222
I is now 12


  0. ▒"▒
  1. 22
  2. 333
  3. 4444
  4. 55555
  5. 666666
  6. 7777777
  7. 88888888
  8. 9999999
  9. 0000000000
 10. 11111111111
 11. 222222222

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Jul 23 '16 at 14:28
You allocated only one character to `s` and read many characters to it (what is pointed by it, strictly speaking), so you invoked *undefined behavior*. Allocate enough elements. — MikeCAT, Jul 23 '16 at 14:29
Accessing out-of-range via `a` also invokes *undefined behavior*. — MikeCAT, Jul 23 '16 at 14:30
Just to make sure I understand, malloc would return an address, so adding the ASCII value 'a' is causing it to go out of range as you said, correct? — BestQualityVacuum, Jul 23 '16 at 19:44

Madhusoodan P · Answer 1 · 2016-07-23T14:51:36.443

0

There are lot of redundancies in the code. First of all you are allocating only one byte for string s and reading a string into it which will cause a undefined behavior (mostly the cause of your problem). The same is true for a.

which must be

char **a = (char**)malloc(sizeof(char*) * SOME_CONSTANT);

Next you are allocating one character at a time for each char in an array which can be done in a single line (Your code will be too inefficient. lot of function calls to malloc). Here

for(j=0; j<(size+1); j++)   a[i][j] = (char)(malloc(sizeof(char)));

Which can be

a[i] = (char*)(malloc(sizeof(char)  * (size+1)));

edited Jul 23 '16 at 14:51

answered Jul 23 '16 at 14:39

Madhusoodan P

681
9
20

Thanks for your reply. I tried what you said, using allocating more than one char for s, by just multiplying it with a constant. I did the same for array pointer 'a' with 10000 elements, and it works. However, I'm still confused as to why I needed to do that for 'a'. Initially, I gave it one address to a char *. In my loop, I call malloc to a[i] to dynamically add more entries. If this is the case, since I'm doing it dynamically, why do I need to use sizeof(char *) * CONSTANT when I first declare **a? – BestQualityVacuum Jul 23 '16 at 19:48

score 0 · Answer 2 · answered Jul 23 '16 at 16:21

You have not allocated enough memory.

char *s = (char *) malloc(sizeof(char));
...
while( scanf("%79s",s) == 1){

You allocate 1 byte, sizeof char, and then read 79 bytes into that address. It will not go well for those other 78 bytes. The fact that anything works at all is pure luck of the draw; it only so happens that you're not using any of the memory that was inadvertently overwritten.

C not storing first entry in array correctly

2 Answers2