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]

Quote:

---

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.

Quote:

---

Originally Posted by Salem

> 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.

---

so is

Code:

---

        another quick question

Code:

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

---

not needed?

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;
  void *temp;
  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';
        }
    }
   
    if(strncmp(input, "exit", 4) == 0)
    {
        stillGoing = FALSE;
        break;
    }
       
    num_of_inputs++;
    cols = strlen(input) +1;
   
    printf("maybe here (inputs realloc)\n");
    temp = realloc(Inputs, num_of_inputs * sizeof(char *));
   
    if(temp)
    {
        Inputs = temp;
        Inputs[num_of_inputs - 1] = input;
    }
    else
    {
        /*handle error*/
        printf("error\n");
        exit(EXIT_FAILURE);
    }
   
   
   
   
    /*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);*/

     
      free(input);
      input = NULL;
      free(temp);
  }
 
 
  int z = 0;
  for(z=0; i < num_of_inputs-1; z++)
  {
      printf("%s\n", Inputs[z]);
  }
 
  free(Inputs);
  return 0;
}

---

new code but it still prints out junk then seg faults

I'm just wondering if you actually try anything anyone else posts, or just find new and interesting ways to screw up your own code.

Here, try this

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;
  void *temp = NULL;
  char *input = NULL;
  Bool stillGoing = TRUE;
  int num_of_inputs = 0;
  int i = 0;

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

    /* read a line, and blow away the newline */
    readReturn = getline(&input, &bufLen, stdin);
    if ( input[ strlen(input) - 1 ] == '\n' ) {
      input[ strlen(input) - 1 ] = '\0';
    }

    /* time to quit yet? */
    if(strncmp(input, "exit", 4) == 0)
    {
      stillGoing = FALSE;
      break;
    }

    /* save the pointer getline() allocated for us */
    num_of_inputs++;
    temp = realloc(Inputs, num_of_inputs * sizeof(char *));
    if(temp)
    {
      Inputs = temp;
      Inputs[num_of_inputs - 1] = input;
    }
    else
    {
      /*handle error*/
      printf("error\n");
      exit(EXIT_FAILURE);
    }

    /* make getline call malloc again for the next line */
    input = NULL;
  }


  /* print and free each line */
  for(i=0; i < num_of_inputs-1; i++)
  {
    printf("%s\n", Inputs[i]);
    free(Inputs[i]);
  }

  /* free all the pointers */
  free(Inputs);
  return 0;
}

---