Loading a file of words into dynamic char**

My goal is to load a char** from a file of words so that there is no wasted space (ie custom amount of cols for the exact amount of letters in each word). I am getting "bus error" when trying to run the program, gdb says:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x00001ea5 in loadArray (inFileName=0xbffffa0f "10-words.txt", array=0xbffff8dc, count=0xbffff8d8, capacity=0xbffff8d4) at SL2.c:68
68 *array[*count] = resizedtmp;


I do not know what this means, but I am assuming it is a bad use of malloc/pointers? I tried referring to array without the *, and this removed bus error and replaced it with a segmentation fault and a warning during compilation. I do not undersand why this error is occurring...I am new to C and have been working on this program all day, something that should have taken an hour or two tops. Thank you very much for your much needed help.

Code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h> 

int loadArray(char *, char ***, int *, int *);
int printArray(char**, int);

int main(int argc, char* argv[]) { 
	  char** A;
	  int size, capacity;
	  loadArray(argv[1], &A, &size, &capacity);
	  printArray(A, size);
	return EXIT_SUCCESS; 
} 

int loadArray(char *inFileName, char ***array, int *count, int *capacity) {
	/* complete this function. The function takes a file name (string)
	   the address of a char** (array of char*), return the size of the
	   file through count (how many words), and also the capacity (space allocated). Allocate 10 spaces 
	   for the array of char* first and then double the size of the 
	   array as needed. You dont need to sort the names. You need sort
	   in lab2 though.
	   Return 0 if the function successfully completes the allocation.
	   Else return -1 to indicate an error.
	*/
	*capacity = 10; /* current capacity */
	*count = 0; /* current count */
	int i = 0;
	int length = 32;
	char tmp[length];
	FILE* fp;
	*array = (char**)malloc(*capacity*sizeof(char*));
	if (*array == NULL) { /* malloc failed */
		printf("Malloc failed; exiting program.\n\n");
		return -1;
	}
	fp = fopen(inFileName, "r");
	if (fp == NULL) { /* File does not exist */ 
		printf("File does not exist; exiting program.\n\n");
		return -1;
	}
	while (fscanf(fp, "%s", &tmp) > 0) { /* add words to array */
		if (*count >= *capacity) { /* array needs to be resized */
			if (realloc(*array, 2*(*capacity)) != NULL) {
				realloc(*array, 2*(*capacity));
				*capacity *= 2;
			}
			else { /* realloc failed */
				printf("Realloc failed; exiting program.\n\n");
				return -1;
			}
		}
		char* resizedtmp;
		resizedtmp = (char *)malloc((strlen(tmp)+1)*sizeof(char)); /* +1 for \0 null character */
		if (resizedtmp == NULL) {
			printf("Malloc failed; exiting program.\n\n");
			return -1;
		}
		strcpy(resizedtmp, tmp);
		*array[*count] = resizedtmp;
		(*count)++;
	}	
	printf("The count is %d words\nThe capacity is %d bytes\n", *count, *capacity);
	return 0;
}

int printArray(char** array, int size) {
	/* prints the file, each word separated by a space. */
	int j = 0;
	for (j; j<=size; j++) {
		printf("%s ", array[j]);
	}
	return 0;
}

Your understanding of realloc() is flawed. realloc() returns a pointer to the block of memory that it deems as being valid for your purposes. It is not required to actually resize the existing block, but it could actually return a pointer to a new block of memory that has all of your data from your original block copied to it.

> int j = 0;
> for (j; j<=size; j++) {
In addition to what MacGyver said, this loop executes one too many times. It should be:
int j = 0;
for (j; j<size; j++) {
Or even simpler:
for (j=0; j<size; j++) {

I see. The loop was a simple mistake and understood. As for realloc, do this mean I need to declare a new char ** and set the realloc equal to it, ie
char ** resizedarray;
resizedarray = realloc(etc);

But then how do I set the original array equal to the resized array and free the space so there is only one array?

Originally Posted by strakerc

I see. The loop was a simple mistake and understood. As for realloc, do this mean I need to declare a new char ** and set the realloc equal to it, ie
char ** resizedarray;
resizedarray = realloc(etc);

Yes.

Originally Posted by strakerc

But then how do I set the original array equal to the resized array and free the space so there is only one array?

Code:

char **tmp = realloc(original_ptr, newsize);
if(tmp != NULL)
{
    original_ptr = tmp;
}

You obviously should handle the other case if the reallocation failed, but this should give you an idea of how to handle it.

That makes sense, although for some reason I thought you were not allowed to just set an array equal to another: should I add free(original_array); ? Regardless, I did these suggestions and it compiled, but I still received the same "bus error." Do you know what else might be wrong with my code? I think it is the line where I set
*array[*count] = resizedtmp;
Thanks for all of your help!

>do this mean I need to declare a new char ** and set the realloc equal to it
That's the preferred method, because in case realloc() returns NULL, you can still free the original before returning from the function.

Code:

			char **resizedarray;
			resizedarray = realloc(etc);
			if (resizedarray != NULL)
			{
			}
			else { /* realloc failed */
				printf("Realloc failed; exiting program.\n\n");
				free(*array);
				return -1;
			}

Or in your case, it appears you print the array anyway, even if an error occurs, so you'd leave it up to main() to free the memory, so you can take out the free(*array) above.

I do catch the realloc failing, but I am still receiving "bus error". See my above post. Also, shouldn't I free the original array regardless and set it equal to resizedarray?

You're not allowed to set an array equal to another one, but there is a difference between a pointer that points to a block of memory and an array that is an array upon declaration. At the hardware level, a difference may or may not exist, but in C, there is definitely a type difference.

Case in point, I would lose the & in front of tmp in your fscanf() call, but for that matter, I would use fgets() instead of fscanf() to read strings since it may be trashing the buffer.

Your entire program is a little messy with pointers and such, making it difficult to tell exactly where you went wrong. I would try breaking down the problem down smaller and trying to simplify the entire thing. You might want to write at least one helper function to cut down on the size and complexity of loadArray().

>Also, shouldn't I free the original array regardless and set it equal to resizedarray?
No, because resizedarray may actually point to the exact same memory.

I may be wrong about this, but I believe if realloc() returns a pointer to new memory, it will free the old memory.

Also I'm assuming you set *array to point to resizedarray at some point. And MacGyver makes a good point about & in front of tmp for the scanf().

Thanks. All you help got this working. Additionally, I needed () around *array.