Allocating memory dynamically to a 2-d array of strings

Question

int ReadNames(char ***Names, int *r,int *c) 
{
int i, j, k;
char name[100];
printf("Number of Rows: ");
scanf("%d", r);
printf("Number of Columns: ");
scanf("%d", c);
Names=(char ***)malloc(sizeof(char **)*(*r));
for(i=0;i<(*r);i++)
*(Names+i)=(char **)malloc(sizeof(char *)*(*c));
for(i=0;i<(*r);i++)
for(j=0;j<(*c);j++)
{
fflush(stdin);
gets(name);
strcpy(*(*(Names+i)+j),name);
}
return 1;
}

I am trying to allocate the memory to a 2-D array of strings. Later on i want to sort them row wise and column wise, but while allocating the memory , the program is not responding. Is there something i am doing in my code.

in main function readname is called as

 ReadNames(&p,&r,&c)

where r and c are the no. of rows and columns.

• John3136: the program is not responding - run it in a debugger or stick some printfs in to see where it is getting stuck.
• Rakesh_Kumar: @John3136 while copying the string it is getting stucked...i.e. in the line "strcpy(*(*(Names+i)+j),name);"
• Thomas: Indentation, please. Do you not care about other people just enough to at least spare them the pain of reading unindented code? Geez.. seriously, how much is that to ask?
• Rakesh_Kumar: sry for the bad indentation...next time it won't happen
• AndersK: always check values that are entered and return values from functions you call to eliminate error sources and initialize all variables when you declare them.

Answer 1

(削除) You need:

*Names = (char **)malloc(sizeof(char **) * (*r));

and consequential changes.

You're passing a triple-pointer in order to be able to return a double-pointer. What you're doing is losing the information about where to store the double-pointer. (削除ここまで)

There is some truth to the struck out comment; there is also a miscue. A 2D array of strings means that you have three levels of pointer in the basic data. And you need a fourth level of pointer to pass into the function.

Also, using gets() is a recipe for disaster. Don't ever (as in never, as in never ever) use the the gets() function. Not even in toy programs. It gets you into bad habits. The first Internet worm propagated through a program that used gets() (Google search 'morris internet worm').

On Unix and other POSIX-based systems, using fflush(stdin) leads to undefined behaviour. On Windows, the behaviour is defined by Microsoft. If you're running on Windows, then you're OK; if not, you're not.

---

And I thought Three-Star Programming was bad!

This probably isn't the way I'd do it, but it is a pretty direct translation of what you wrote into something that works, along with a main() program that tests it and frees all the allocated memory. It assumes strdup() is available; if not, it is trivial to write it.

Sample output:

Number of Rows: 2
Number of Columns: 3
R0C0: Row 1, Column 1.
R0C1: Ambidextrous Armless Individual.
R0C2: Data for the third column of the first row.
R1C0: Row 2, Column 1.
R1C1: Row 2, Column 2.
R1C2: Given that the number of rows is 2 and the number of columns is 3, this should be the last input!
Rows = 2, cols = 3.
[0,0] = <<Row 1, Column 1.>>
[0,1] = <<Ambidextrous Armless Individual.>>
[0,2] = <<Data for the third column of the first row.>>
[1,0] = <<Row 2, Column 1.>>
[1,1] = <<Row 2, Column 2.>>
[1,2] = <<Given that the number of rows is 2 and the number of columns is 3, this should be the last input!>>

Working 4-star code:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
static void ReadNames(char ****Names, int *rows, int *cols)
{
 char name[100];
 printf("Number of Rows: ");
 scanf("%d", rows);
 printf("Number of Columns: ");
 scanf("%d", cols);
 int c;
 while ((c = getchar()) != EOF && c != '\n')
 ;
 *Names = (char ***)malloc(sizeof(char ***)*(*rows));
 for (int i = 0; i < (*rows); i++)
 (*Names)[i] = (char **)malloc(sizeof(char **)*(*cols));
 for (int i = 0; i < (*rows); i++)
 {
 for (int j = 0; j < (*cols); j++)
 {
 printf("R%dC%d: ", i, j);
 if (fgets(name, sizeof(name), stdin) == 0)
 {
 fprintf(stderr, "Unexpected EOF\n");
 exit(1);
 }
 name[strlen(name)-1] = '0円'; // Zap newline
 (*Names)[i][j] = strdup(name);
 }
 }
}
int main(void)
{
 int rows;
 int cols;
 char ***data = 0;
 ReadNames(&data, &rows, &cols);
 printf("Rows = %d, cols = %d.\n", rows, cols);
 for (int i = 0; i < rows; i++)
 {
 for (int j = 0; j < cols; j++)
 printf("[%d,%d] = <<%s>>\n", i, j, data[i][j]);
 }
 for (int i = 0; i < rows; i++)
 {
 for (int j = 0; j < cols; j++)
 free(data[i][j]);
 free(data[i]);
 }
 free(data);
 return 0;
}

Alternative 3-star code

Using three levels of pointer is bad enough; four is horrid. This code restricts itself to three levels of pointer. I assume C99 compatibility, so variables can be declared when convenient in a function. The changes to work with C89/C90 compilers (which are 14 years retrograde now) are simple enough.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
static char ***ReadNames(int *r, int *c)
{
 int i, j;
 char name[100];
 printf("Number of Rows: ");
 scanf("%d", r);
 printf("Number of Columns: ");
 scanf("%d", c);
 int x;
 while ((x = getchar()) != EOF && x != '\n')
 ;
 char ***Names = (char ***)malloc(sizeof(char ***)*(*r));
 for (i = 0; i < (*r); i++)
 Names[i] = (char **)malloc(sizeof(char **)*(*c));
 for (i = 0; i < (*r); i++)
 {
 for (j = 0; j < (*c); j++)
 {
 if (fgets(name, sizeof(name), stdin) == 0)
 {
 fprintf(stderr, "Unexpected EOF\n");
 exit(1);
 }
 name[strlen(name)-1] = '0円';
 Names[i][j] = strdup(name);
 }
 }
 return Names;
}
static void PrintNames(char ***Names, int r, int c)
{
 int i, j;
 for (i = 0; i < r; i++)
 {
 for (j = 0; j < c; j++)
 printf("%s ", Names[i][j]);
 printf("\n");
 }
}
int main(void)
{
 int rows;
 int cols;
 char ***data = ReadNames(&rows, &cols);
 PrintNames(data, rows, cols);
 printf("Rows = %d, cols = %d.\n", rows, cols);
 for (int i = 0; i < rows; i++)
 {
 for (int j = 0; j < cols; j++)
 printf("[%d,%d] = <<%s>>\n", i, j, data[i][j]);
 }
 for (int i = 0; i < rows; i++)
 {
 for (int j = 0; j < cols; j++)
 free(data[i][j]);
 free(data[i]);
 }
 free(data);
 return 0;
}

Example output

Number of Rows: 3
Number of Columns: 4
R1C1
R1C2
R1C3
R1C4-EOR
R2C1
R2C2
R2C3
R2C4-EOR
R3C1
R3C2
R3C3
R3C4-EOR
R1C1 R1C2 R1C3 R1C4-EOR 
R2C1 R2C2 R2C3 R2C4-EOR 
R3C1 R3C2 R3C3 R3C4-EOR 
Rows = 3, cols = 4.
[0,0] = <<R1C1>>
[0,1] = <<R1C2>>
[0,2] = <<R1C3>>
[0,3] = <<R1C4-EOR>>
[1,0] = <<R2C1>>
[1,1] = <<R2C2>>
[1,2] = <<R2C3>>
[1,3] = <<R2C4-EOR>>
[2,0] = <<R3C1>>
[2,1] = <<R3C2>>
[2,2] = <<R3C3>>
[2,3] = <<R3C4-EOR>>

Both programs run clean under valgrind.

• Rakesh_Kumar: Leffer.....what is wrong in doing this Names=(char ***)malloc(sizeof(char **)*(*r)); Names contains the base address of array having data types as doubly pointers...i.e. Names[i]
• Jonathan Leffler: It doesn't do what you seem to think it does. In particular, it does not alter the pointer in the calling function, and once you've made your assignment, you have no way of changing the pointer in the calling function since you've zapped the pointer to that pointer.
• Jonathan Leffler: Alternatively, your trouble is that you're not passing a char **** to the function...and you're not allocating enough space for the strings. Supposing the user type 3 rows and 2 columns; the user is then expected to type 6 lines of data which will be stored and accessible in the calling function?
• Rakesh_Kumar: @Jonathan..i just worked out on the problem as per your explaination..storing the strings is fine..but printing is creating a problem again..I have shared the code...wat else could be a problem?

Answer 2

So, this is the code working...

 *Names=(char **)malloc(sizeof(char **)*(*r)); 
for(i=0;i<(*r);i++) 
 *(*Names+i)=(char*)malloc(sizeof(char *)*(*c)); 
for(i=0;i<(*r);i++)
for(j=0;j<(*c);j++)
{
fflush(stdin);
gets(name);
strcpy((*(*Names+i)+j),name);
}

but while printing those names stored...i have a function provided..

int PrintNames(char **Names, int r, int c)
{
int i,j;
for(i=0;i<r;i++)
{ printf("\n");
 for(j=0;j<c;j++)
 printf("%s ",*(*(Names+i)+j));
}
return 1;
}

Now this PrintNames is also called through main...as "PrintNames(p, r, c);"....but program stops while printing...What could be wrong?

• Jonathan Leffler: This isn't an answer; it belongs in the question as an amendment. Leave the original code intact, but add this too. Why do people like using *(*(Names+i)+j) instead of the simpler Names[i][j]?
• Rakesh_Kumar: @JonathanLeffler well using Names[i][j] is not solving the problem either!
• Jonathan Leffler: In your PrintNames(), I get a compiler warning: 2dsb.c: In function ‘PrintNames’: 2dsb.c:36:13: error: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘int’ [-Werror=format=] printf("%s ", *(*(Names+i)+j)); This indicates a problem. I realized (eventually), that you do need a triple pointer for the data in main() and you would need it in PrintNames(); you need a quadruple pointer in ReadNames() while you pass in the address of the triple pointer; you could simplify things if you return a triple pointer (and don't take a pointer argument). Etc.