Dynamically allocating 2d arrays

[bondo]

Basically I'm reading from a file and populating a dynamically allocated array with the words that are in the file. I've already read through the file to figure out what numArts is, so I know how many words there are in the array.

This works up until the:

arts[index] = (char*)malloc(strlen(word) * sizeof(char));
		strcpy(*(arts + index), word);

if I remove those lines, the printf statment prints what I expect it to. I've tried doing this with pointer arithmetic as well. I just keep getting bus error over and over. From what I can tell and what I've been able to read about dynamically allocating arrays and such I'm doing this correctly. Does anyone see anything wrong with this?

void populateArts(char **arts, FILE *fin, int *numArts)
{
	int index;
	char* word; /* Temporary holding for each word */
	
	arts = malloc(*numArts * sizeof(char*));
	
	fscanf(fin, "%s", word); /* Skip over category */
	/* Fill articles array */
	for(index = 0; index < *numArts; index++)
	{
		fscanf(fin, "%s", word);
		printf("word is %s\n", word);
		
		arts[index] = (char*)malloc(strlen(word) * sizeof(char));
		strcpy(*(arts + index), word);
	}
}

[Narue]

>char* word; /* Temporary holding for each word */
>fscanf(fin, "%s", word);
Pointers aren't magic containers for infinite data. You need to allocate space to word before you can write to it. In this case, there's little point in using a pointer unless you want to get fancy with single character reads.

>arts[index] = (char*)malloc(strlen(word) * sizeof(char));
You cast the result of malloc in one call but not another. Consistency is more important than perfect style, but I'd still recommend removing the cast. Also, instead of sizeof(char) , you can get the same effect without specifying the type directly using sizeof *arts[index] . Also note that sizeof(char) is guaranteed to be 1.

Finally, when allocating memory for a string, you need to add an extra slot for the null character. If you don't do that, you'll end up with problems later when overflowing the buffer.

Try this:

void populateArts(char **arts, FILE *fin, int *numArts)
{
	int index;
	char word[BUFSIZ]; /* Temporary holding for each word */
	
	arts = malloc(*numArts * sizeof *arts);
	
	fscanf(fin, "%s", word); /* Skip over category */

	/* Fill articles array */
	for(index = 0; index < *numArts; index++)
	{
		fscanf(fin, "%s", word);
		printf("word is %s\n", word);
		
		arts[index] = malloc(strlen(word) + 1);
		strcpy(arts[index], word);
	}
}

[bondo]

Ah! I see. Fantastic. Thanks so much. I know pointers aren't "magic containers", but the only reason I was trying

char* word;
fscanf(fin, "%s", word)

is that earlier on while reading through the file I am doing this:

int countWords(FILE* fin)
{
	int count = 0;
	char* temp; /* Temporary holding for each line */
	fscanf(fin, "%s", temp); /* Read the category name, but don't count it */
	
	while(*temp != '#' && !feof(fin))
	{
		fscanf(fin, "%s", temp);
		printf("temp is %s\n", temp);
		if(*temp != '#')
			count++;
	}
	
	return count;
}

Which, to me, looked exactly the same as what I was doing in the other function, except for the allocating memory part, so I was getting confused as to why in countWords() I am able to read into a pointer that didn't have memory allocated to it and print out the correct results, but in populateArts() it wasn't working. Do you have some insight as to why it was working in one place, but not in another? I just would like to have the concept clarified, if you don't mind.

Thanks again!

Oh also, how do you specify what kind of syntax you are using with the CODE tags?

[Nick Evan]

Oh also, how do you specify what kind of syntax you are using with the CODE tags?

for c:

[CODE=C] /*code goes here */

[/code]

also explained here

[Narue]

>Do you have some insight as to why it was working in one place, but not in another?
You got unlucky where it worked and lucky in another because bugs are more obvious when they break immediately. As for the detailed why, the random address as temp's value in countWords happened to be in your address space and writing to it happened not to cross any boundaries that would cause a segmentation fault. In populateArts, the random address wasn't nearly as benign.

>while(*temp != '#' && !feof(fin))
For future reference, feof isn't designed to be used as a loop condition like that. Because the end-of-file indicator for a stream is only set after you've tried and failed to read from the stream, loops constructed like this have an off-by-one error where the last record in the stream will be processed twice. This is easily tested and proven with a simple program:

#include <stdio.h>

int main ( void )
{
  char buf[BUFSIZ] = "I'm a monkey! Ook ook!\n";

  while ( !feof ( stdin ) ) {
    fgets( buf, sizeof buf, stdin );
    printf ( "Processing a line: %s", buf );
  }

  return 0;
}

[bondo]

Oh, I see. So really it was wrong in BOTH places, it just happened to be work in one place and not in the other, but really I should have done the both how you said. Awesome.

Thanks again!