Dynamically Allocating a 2D Array.

Code:

#define _GNU_SOURCE 1
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <getopt.h>
#include <sys/types.h>
#include <sys/wait.h>

typedef enum
{
   FALSE = 0,
   TRUE = 1
} Bool;

int main()
{
   ssize_t readReturn = 0;
   size_t bufLen = 0;
   char **Inputs = NULL;
   char *input = NULL;
   int x = 0;
   Bool stillGoing = TRUE;
   int num_of_inputs = 0;
   int cols = 0;
   int old_cols = 0;
   int i = 0;

   while(stillGoing)
   {
     printf("enter text: ");
   
     readReturn = getline(&input, &bufLen, stdin);
   
     for(x = 0; x < strlen(input) + 1; ++x)
     {
        if(input[x] == '\n')
        {
           input[x] = '\0';
        }
     }
     
     num_of_inputs++;
     cols = strlen(input) +1;
     
     printf("maybe here (inputs realloc)\n");
	 Inputs = realloc(Inputs, num_of_inputs * sizeof(char *));

     if(cols > old_cols)
      {
         for(i = 0; i < num_of_inputs; i++)
         {
            Inputs[i] = malloc(cols * sizeof(char));
         }
         old_cols = cols;
      }     

      printf("maybe this is where it's seg faulted (strcpy)\n");
	  strcpy(&Inputs[num_of_inputs - 1][0], input);

      if(strncmp(input, "exit", 4) == 0)
      {
         stillGoing = FALSE;
      }
	  free(input);
	  input = NULL;
   }
   
   free(Inputs);
   return 0;
}

i keep geting a seg fault i have no idea why. the program is supposed to add the user's input into a new row in the array. The array will end up looking like this: http://www.rafb.net/paste/results/9YpiYn61.html

I haven't checked in exhaustive detail but, reading from the top of main(), I see the start of the program involves the following steps;

Code:

char *input = NULL;
/*  more stuff, but input not changed */
readReturn = getline(&input, &bufLen, stdin);

Firstly, getline requires a pointer to char, not a pointer to pointer (most compilers will give a warning on this, ignoring such warnings is not a good idea).

Second, as input is a NULL pointer, and getline will attempt to write to memory it points to, the result will be undefined behaviour (regardless of whether you pass input or &input as an argument). One common symptom of undefined behaviour is a segmentation fault.....

Since getline() is not a standard function, you can't be sure whether it's being called correctly. Since the OP defined _GNU_SOURCE it's reasonable to expect that he's on a GNU system, and using GNU's getline(), which is prototyped as follows:

Code:

ssize_t getline(char **lineptr, size_t *n, FILE *stream)

Moreover, GNU getline() allows *lineptr to be NULL, in which case it will allocate memory as necessary.

That's not to say that I necessarily advocate non-standard functions, of course ...

if anyone is interested i fixed the problem. Here's the new code:

Code:

#define _GNU_SOURCE 1
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <getopt.h>
#include <sys/types.h>
#include <sys/wait.h>

typedef enum
{
   FALSE = 0,
   TRUE = 1
} Bool;

int main()
{
   ssize_t readReturn = 0;
   size_t bufLen = 0;
   char **Inputs = NULL;
   char *input = NULL;
   int x = 0;
   Bool stillGoing = TRUE;
   int num_of_inputs = 0;
   int cols = 0;
   int old_cols = 0;
   int i = 0;

   while(stillGoing)
   {
     printf("enter text: ");
   
     readReturn = getline(&input, &bufLen, stdin);
   
     for(x = 0; x < strlen(input) + 1; ++x)
     {
        if(input[x] == '\n')
        {
           input[x] = '\0';
        }
     }
     
     num_of_inputs++;
     cols = strlen(input) +1;
     
     printf("maybe here (inputs realloc)\n");
	 Inputs = malloc(num_of_inputs * sizeof(char *));

     /*if(cols > old_cols)
      {*/
         for(i = 0; i < num_of_inputs; i++)
         {
            Inputs[i] = malloc(cols * sizeof(char));
         }
         old_cols = cols;
      /*}*/     

      printf("maybe this is where it's seg faulted (strcpy)\n");
	  strcpy(Inputs[num_of_inputs - 1], input);

      if(strncmp(input, "exit", 4) == 0)
      {
         stillGoing = FALSE;
      }
	  free(input);
	  input = NULL;
   }
   
   free(Inputs);
   return 0;
}

> The array will end up looking like this: http://www.rafb.net/paste/results/9YpiYn61.html
Why did you split your post across two boards?
Especially since one of them is 404'ed at the moment.

> for(x = 0; x < strlen(input) + 1; ++x)
As with fgets(), won't the newline be at the end of the string, if it's there at all?
Eg if ( input[ strlen(input)-1 ] == '\n' )
Calling strlen() every time around the loop is surely a waste.

> Inputs = realloc(Inputs, num_of_inputs * sizeof(char *));
Classic realloc mis-use bug.
If realloc fails, you've just trashed your only pointer, but the old memory is still allocated.
Search for my previous posts on the use of realloc

> Inputs[i] = malloc(cols * sizeof(char));
So every time you type in a longer line than before (if(cols > old_cols)), you trash all the memory allocations for the previous input lines???

Since getline does an alloc for you, you can save a whole bunch of malloc and strcpy stuff by simply doing

Code:

void *temp;
while(stillGoing)
{
  printf("enter text: ");
  fflush(stdout);

  readReturn = getline(&input, &bufLen, stdin);

  num_of_inputs++;
  temp = realloc(Inputs, num_of_inputs * sizeof(char *));
  if ( temp ) {
    Inputs = temp;
  } else {
    /* error? */
  }

  /* save the pointer for the current line */
  Inputs[num_of_inputs - 1] = input;

  if(strncmp(input, "exit", 4) == 0)
  {
    stillGoing = FALSE;
  }
  input = NULL;  /* pointer is saved */
}

Edit:
> if anyone is interested i fixed the problem. Here's the new code:
No you didn't - now you're not allocating ANYTHING for your strcpy to work with.

See this FAQ on multidimensional array allocation.

^yea i used that faq to formulate the new code. i dont understand what the other guy was saying is wrong with the new code.

> I dont understand what the other guy was saying is wrong with the new code.
> strcpy(Inputs[num_of_inputs - 1], input);
Where is this copying to?
You only allocated space for one extra pointer, not a whole string.

would doing:

Code:

Inputs[num_of_inputs -1] = strdup(input);

instead of the strcpy statement work?

I have doubts on how this works though. I want every element in the array to hold a single character. So that every row in the 2d array will form the string. This seems to be putting a string in every element which is not what I planned to do. I wondering now if it is more beneficial to use a linked list here.

> Inputs[num_of_inputs -1] = strdup(input);
Sure, if you have such a function.
But strdup() is just a thin wrapper around malloc() and strcpy().

getline(&input, &bufLen, stdin); // calls malloc
Inputs[num_of_inputs -1] = strdup(input); // calls malloc again
free( input ); // returns the first malloc back to the pool

Seems more wasteful than simply letting getline() do all the allocating, and you just saving each pointer
getline(&input, &bufLen, stdin);
Inputs[num_of_inputs - 1] = input;

> I want every element in the array to hold a single character.
Which is what you will have.
Inputs[0] is the whole of the first line you type in
Inputs[0][0] is the first character of the first line you type in

another quick question

Code:

for(i = 0; i < num_of_inputs; i++)
         {
            Inputs[i] = malloc(cols * sizeof(char));
         }

isnt this allocating space for the whole string? if it is i dont see why the strdup is neccessary.

NO!
It's trashing all your previous strings.

if i did a realloc would it still trash all my previous strings?

[sidenote] sizeof(char) is guarenteed to be 1. You don't need it there. [/sidenote]

if i did a realloc would it still trash all my previous strings?

Yeah, I think you need another variable.

> if i did a realloc would it still trash all my previous strings?
Your realloc allocates one more char * pointer, not a string.
All your existing strings (the pointers AND the memory they point to) are preserved. You don't need to do any more work with the strings you already have.