Allocating multidimensional char pointer in C

Question

I am trying to read lines from a file and store them in a multidimensional char pointer. When I run my code, it runs without errors, however, the lines will not be printed correctly in my main() function, but prints correctly in the getline() function. Can anybody explain what is happening and how I correctly store them in my multidimensional pointer?

My code:

int main (int argc, const char * argv[]) {
    char **s = (char**) malloc(sizeof(char*) * 1000);
    int i = 0;
    while (getline(s[i]) != -1){
        printf("In array: %s \n", s[i]);
        i++;
    }
    return 0;
}

int getline(char *s){
    s = malloc(sizeof(char*) * 1000);
    int c, i = 0;
    while ((c = getchar()) != '\n' && c != EOF) {
        s[i++] = c;
    }
    s[i] = '\0';
    printf("String: %s \n", s);
    if (c == EOF) {
        return -1;
    }
    return 0;
}  

and my output:

String: First line
<br>In array: (null) 
<br>String: Second line 
<br>In array: (null) 
<br>String: Third line 
<br>In array: (null) 

Answer 1

You are passing by value. Even though you change the value of *s in getline(), main does not see it. You have to pass the address of s[i] so that getline() can change it:

int getline(char **s) {
  * s= malloc( sizeof(char) * 1000) ;
  ...
}

Also, if you want to be a bit more efficient with memory, read the line into a local buffer (of size 1000) if you want. Then when you are done reading the line, allocate only the memory you need to store the actual string.

int getline( char ** s )
{
  char tmpstr[1000] ;
  ...
    tmpstr[i++]= c ;
  }
  tmpstr[i]= '\0' ;

  * s= strdup( tmpstr) ;
  return 0 ;
}

If you want to improve things even further, take a step back and thing about a few things. 1) allocating the two parts of the multi-dimensional array in two different functions is going to make it harder for others to understand. 2) passing in a temporary string from outside to getline() would allow it to be significantly simpler:

int main()
{
  char ** s= (char **) malloc( 1000 * sizeof(char *)) ;
  char tmpstr[1000] ;
  int i ;

  while ( -1 != getline( tmpstr))
  {
    s[i ++]= strdup( tmpstr) ;
  }

  return 0 ;
}

int getline( char * s)
{
  int c, i = 0 ;
  while (( '\n' != ( c= getchar())) && ( EOF != c )) { s[i ++]= c ; }
  s[i]= '\0' ;

  return ( EOF == c ) ? -1 : 0 ;
} 

Now, getline is just about IO, and all the allocation of s is handled in one place, and thus easier to reason about.

Answer 2

The problem is that this line inside getline function

s = malloc(sizeof(char) * 1000); // Should be sizeof(char), not sizeof(char*)

has no effect on the s[i] pointer passed in. This is because pointers are passed by value.

You have two choices here:

• Move your memory allocation into main, and keep passing the pointer, or
• Keep your allocation in getline, but pass it a pointer to pointer from main.

Here is how you change main for the first option:

int main (int argc, const char * argv[]) {
    char **s = malloc(sizeof(char*) * 1000);
    int i = 0;
    for ( ; ; ) {
        s[i] = malloc(sizeof(char) * 1000);
        if (getline(s[i]) == -1) break;
        printf("In array: %s \n", s[i]);
        i++;
    }
    return 0;
}

Answer 3

Why do you use malloc in the first place. malloc is very very dangerous!! Just use s[1000][1000] and pass &s[0][0] for the first line, &s[1][0] for the second line etc.

For printing printf("%s \n", s[i]); where i is the line you want.

Answer 4

My advise: completely avoid writing your own getline() function, and avoid all fixed size buffers. In the POSIX.1-2008 standart, there is already a getline() function. So you can do this:

//Tell stdio.h that we want POSIX.1-2008 functions
#define _POSIX_C_SOURCE 200809L
#include <stdio.h>

int main() {
    char** s = NULL;
    int count = 0;
    do {
        count++;
        s = realloc(s, count*sizeof(char*));
        s[count-1] = NULL;    //so that getline() will allocate the memory itself
    } while(getline(&s[count-1], NULL, stdin));

    for(int i = 0; i < count; i++) printf(s[i]);
}