2D Array Malloc

Question

Below is a part of a longer code where malloc'ing for a 2D array is done. Could anyone tell if this is correct? If I introduce static values, the code works fine. Else, seg faults...

enum { LEN = 1024*8 };
char **tab = NULL;
int cur_LEN = LEN;
int count_lineMax = 0;
tab = malloc(count_lineMax * sizeof(*tab));
 memset(tab, 0, count_lineMax * sizeof(*tab));
if(tab == NULL && count_lineMax) {
 printf("Mem_check\n");
 exit(1);
}
for(k=0;k<count_lineMax;k++) {
 tab[k] = malloc(cur_LEN*sizeof(*tab[k]));
 memset(tab[k], 0, cur_LEN*sizeof(*tab[k]));
 if(tab[k] == NULL) {
 printf("Mem_check*\n");
 exit(1);
 }
}
for(l=0;l<count_lineMax;l++) {
 free(tab[l]);
}
free(tab);

• Eric Z: count_lineMax is zero so you allocate nothing.
• Keith: Looks OK; works for me even when setting count_lineMax. Having said that you could allocate a single block.
• Jason Coco: You are getting a seg fault because you are allocating 0 bytes and not checking (malloc is going to return null every time). If you actually want a 2D array, you can allocate the entire block at once in one line. If you want an array of variable-length C-strings (which is kind of what it looks like you want), you need to malloc the first list with a lineMax > 0 and then allocate enough space at each element for the variable-length C-string at that location (don't forget to include an extra char for '0円')
• Jonathan Leffler: And you need to check the memory allocation before using memset() on it -- and you should consider using calloc() instead of malloc() since it will do the zeroing for you. It also is not clear that you benefit from zeroing tab since the next thing you do is allocate values into the recently zeroed space.
• R.. GitHub STOP HELPING ICE: The data structure you've created is not a 2D array. It's an array of pointers to [the first elements of] 1D arrays.

Answer 1

int count_lineMax = 0;
tab = malloc(count_lineMax * sizeof(*tab));

What is this? You are gonna malloc 0 bytes?

• gyan: I should have added this loop as well: while(fgets(line, cur_LEN, file1) != NULL) { count_lineMax++; }
• Jeegar Patel: have you added that loop befor malloc.??
• gyan: oops! I meant before malloc! Sorry
• Jeegar Patel: still you have problem ?then please compile your code with -g option & run with valgrind. (if you dont know valgrind then search it on google)

Answer 2

There are at least two ways to build the table while reading lines. One uses the property of realloc() that if its first argument is a null pointer, it will behave like malloc() and allocate the requested space (so the code can be self-starting, using realloc() alone). That code might look like:

#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
enum { LEN = 1024*8 };
static void error(const char *fmt, ...);
static char *xstrdup(const char *str);
int main(void)
{
 char line[LEN];
 char **tab = NULL;
 int tabsize = 0;
 int lineno = 0;
 while (fgets(line, sizeof(line), stdin) != 0)
 {
 if (lineno >= tabsize)
 {
 size_t newsize = (tabsize + 2) * 2;
 char **newtab = realloc(tab, newsize * sizeof(*newtab));
 if (newtab == 0)
 error("Failed to allocate %zu bytes of memory\n", newsize * sizeof(*newtab));
 tab = newtab;
 tabsize = newsize;
 }
 tab[lineno++] = xstrdup(line);
 }
 /* Process the lines */
 for (int i = 0; i < lineno; i++)
 printf("%d: %s", i+1, tab[i]);
 /* Release the lines */
 for (int i = 0; i < lineno; i++)
 free(tab[i]);
 free(tab);
 return(0);
}
static void error(const char *fmt, ...)
{
 va_list args;
 va_start(args, fmt);
 vfprintf(stderr, fmt, args);
 va_end(args);
 exit(1);
}
static char *xstrdup(const char *str)
{
 size_t len = strlen(str) + 1;
 char *copy = malloc(len);
 if (copy == 0)
 error("Failed to allocate %zu bytes of memory\n", len);
 memmove(copy, str, len);
 return(copy);
}

The alternative uses malloc() explicitly when the table is empty, and is most simply coded as:

int main(void)
{
 char line[LEN];
 int tabsize = 4;
 int lineno = 0;
 char **tab = malloc(tabsize * sizeof(*tab));
 if (tab == 0)
 error("Failed to allocate %zu bytes of memory\n", tabsize * sizeof(*tab));
 ...

Everything else can remain untouched.

Note that it can be convenient to have functions xmalloc() and xrealloc() which are guaranteed never to return a null pointer because they report an error instead:

static void *xmalloc(size_t nbytes)
{
 void *space = malloc(nbytes);
 if (space == 0)
 error("Failed to allocate %zu bytes of memory\n", nbytes);
 return(space);
}
static void *xrealloc(void *buffer, size_t nbytes)
{
 void *space = realloc(buffer, nbytes);
 if (space == 0)
 error("Failed to reallocate %zu bytes of memory\n", nbytes);
 return(space);
}

In the long term (big programs), this cuts down on the total number of times you write 'out of memory' error messages. On the other hand, if you must recover from memory allocation failures (save the user's work, etc), then this is not an appropriate strategy.

The code above creates a ragged array; different entries in tab have different lengths. If you want homogeneous lengths (as in the original code), then you have to replace or modify the xstrdup() function to allocate the maximum length.

You might have noticed that the code in xstrdup() used memmove() instead strcpy(). That's because strlen() already measured how long the string is, so there's no need for the copy code to test each byte to see whether it needs copying. I used memmove() because it can never go wrong, even if the strings overlap, even though in this context it is clear that the strings can never overlap and so memcpy() - which is not guaranteed to work correctly if the strings overlap - could have been used since the strings cannot overlap.

The strategy of allocating (oldsize + 2) * 2 new entries means that the memory reallocation code is exercised often enough during testing, without unduly impacting performance in production. See The Practice of Programming by Kernighan and Pike for a discussion of why this is a good idea.

I almost always use a set of functions similar to the error() function because it dramatically simplifies error reporting. The functions I normally use are part of a package that records and reports the program name (from argv[0]) as well, and that has a fairly wide range of alternative behaviours.

• gyan: Thanks for all the info. I'd have to print this out and go over all of it carefully. Thanks Again.