[InterNoctem]

I'm trying to write a program to extract data from binary files, to organize the data I used this structure:

typedef struct {
	const char* name;
	int display_lists;

	struct /*dlist[]*/ {
		char* display_data;
		int vtx_calls;

		struct /*vtx[]*/ {
			uint8 start;
			uint16 size;
			uint8 half;
			uint8 bank;
			uint32 offset;

			int vertices;

			char* vertex_data;

			struct {
				int x;
				int y;
				int z;
				int u;
				int v;
				unsigned int r;
				unsigned int g;
				unsigned int b;
				unsigned int a; 
			} vertex[];
		} vtx[];
	}dlist[];
} zobj;

The program scans the file for a pattern of bytes and, once found, loads zobj object.dlist[].display_data one byte at a time until another pattern is found. All goes well until I reach

fread(&object->dlist[j].display_data[k], 1, 1, z_object);

where I get a segmentation fault. I suspect I need to allocate memory to each display_data in the array but I'm unsure how to do this. Do I need to create a loop and malloc() each one, then free() each one at the end of the program? I may have simply gone about the whole thing wrong and over-complicated it. Here's the function giving me trouble.

int load_zobj(const char* filename, zobj* object)
{
	FILE* z_object;
	z_object = fopen(filename, "r");
	object->name = filename;
	long size = 100;
	fseek(z_object, 0, SEEK_END);
	size = ftell(z_object);
	rewind(z_object);
	if(!z_object)
	{
		fprintf(stderr, "Could not open file.\n");
	}

	/* Parse for display lists. */
	unsigned int* buffer = malloc(sizeof(unsigned int));
	object->dlist->display_data = malloc(size); /* This did not appear to work */


	int i = 0;
	int j = 0;
	int k = 0;
	printf("Loading structure.\n");
	while(i < size)
	{
		fseek(z_object, i, SEEK_SET);
		fread(buffer, 8, 1, z_object);

		*buffer = ((*buffer & 0xFF000000) >> 24|(*buffer & 0x00FF0000) >> 8|(*buffer & 0x0000FF00) << 8|(*buffer & 0x000000FF) << 24); /* Byte swap from Big Endian */

		printf("i = %x, and buffer = %08X\n", i, *buffer);
		if(*buffer == 0xE7000000)
		{
			k = 0;
			while(*buffer != 0xDF000000)
			{
				
				printf("i = %x, and buffer = %08X\n", i, *buffer);
				i++;
				fseek(z_object, i, SEEK_SET);
				fread(buffer, 8, 1, z_object);
	//			printf("%i\n", object->dlist[j].display_data[k]);
				fread(&object->dlist[j].display_data[k], 1, 1, z_object);
				*buffer = ((*buffer & 0xFF000000) >> 24|(*buffer & 0x00FF0000) >> 8|(*buffer & 0x0000FF00) << 8|(*buffer & 0x000000FF) << 24);
				k++;
			}
			printf("done\n");
			//i += (k + 8);
			j++;
		}
		i++;
	}
	printf("Parsing data.\n");
	object->display_lists = j;
	/* Get vtx structure from display_data. */
	i = 0;
	while(i < object->display_lists)
	{
		j = 0;
		while(j < sizeof(object->dlist[i].display_data))
		{
			k = 0;
			if((object->dlist[i].display_data[j] == 0x01) && (object->dlist[i].display_data[j+4] == 0x06)) /* Check for vtx command and ram bank 06 request. */
			{
				/* Copy data from display list to struct. */
				object->dlist[i].vtx[k].start = object->dlist[i].display_data[j];
				memcpy(&object->dlist[i].vtx[k].size, &object->dlist[i].display_data[j+1], 2);
				object->dlist[i].vtx[k].half = object->dlist[i].display_data[j+3];
				object->dlist[i].vtx[k].bank = object->dlist[i].display_data[j+4];
				memcpy(&object->dlist[i].vtx[k].offset, &object->dlist[i].display_data[j+5], 3);
				k++;
			}
			object->dlist[i].vtx_calls = k;
		} 
		i++;
	}
	printf("Loading vertex data.\n");
	/* load vertex data */
	i = 0;
	while(i < object->display_lists)
	{
		j = 0;
		while(j < object->dlist[i].vtx_calls)
		{
			/* Load vertex_data. */
			fseek(z_object, object->dlist[i].vtx[j].offset, SEEK_SET);
			fread(object->dlist[i].vtx[j].vertex_data, object->dlist[i].vtx[j].size, 1, z_object);
			/* Load vertex[]. */
			k = 0;
			while(k < (object->dlist[i].vtx[j].half/2)) /* Indicates the number of vertices. */
			{
				/* Copy vertex data from offset (k), 16 bytes. */
				memcpy(&object->dlist[i].vtx[j].vertex[k].x, &object->dlist[i].vtx[j].vertex_data + k, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].y, &object->dlist[i].vtx[j].vertex_data + k + 2, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].z, &object->dlist[i].vtx[j].vertex_data + k + 4, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].u, &object->dlist[i].vtx[j].vertex_data + k + 8, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].v, &object->dlist[i].vtx[j].vertex_data + k + 10, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].r, &object->dlist[i].vtx[j].vertex_data + k + 12, 1);
				memcpy(&object->dlist[i].vtx[j].vertex[k].g, &object->dlist[i].vtx[j].vertex_data + k + 13, 1);
				memcpy(&object->dlist[i].vtx[j].vertex[k].b, &object->dlist[i].vtx[j].vertex_data + k + 14, 1);
				memcpy(&object->dlist[i].vtx[j].vertex[k].a, &object->dlist[i].vtx[j].vertex_data + k + 15, 1);
				k = k + 16;
			}
		}
		i++;
	}
	printf("Finished.\n");
	free(object->dlist->display_data = malloc(size));
	free(buffer);
	fclose(z_object);
	return(0);
}

[baavgai]

So, you kind of have:

typedef struct {
	int x; /* ok, a value */
	struct { /* nested, anonymous, struct */
		int y;
	} bar[]; /* a variable named bar, but WTF does [] mean? */
} Foo;

Dial it back a bit. Your first real structure is:

typedef struct {
	int x;
	int y;
	int z;
	int u;
	int v;
	unsigned int r;
	unsigned int g;
	unsigned int b;
	unsigned int a; 
} Vertex;

The next:

typedef struct {
	uint8 start;
	uint16 size;
	uint8 half;
	uint8 bank;
	uint32 offset;

	int vertices;

	char* vertex_data;

	/* this is meaningless, but is the intent? */
	Vertex vertex[];
} Vtx;

You seem to want collections. Make a struct to represent each:

typedef struct {
	Vertex *items;
	int size;
} Vertexes;

[InterNoctem]

You're right, I got in over my head trying to nest the structs. I'm working on rewriting that now, thanks for clearing it up for me.
As far as the actual program I've found that it will successfully run through the loop once, but segfaults when the variable j is incremented, the line

object->dlist->display_data = malloc(size)

is actually working, and is allocating the first space for object.dlist[], but not the second. I think I'm wondering how I would accomplish

object->dlist[j]->display_data = malloc(size)

as I recieve errors when compiling that literally.

[baavgai]

size = ftell(z_object);
/* note, this is the size of an unsigned int... */
unsigned int* buffer = malloc(sizeof(unsigned int));

/* considering zobj isn't really valid, you aren't going anywhere this this */
object->dlist->display_data = malloc(size); /* This did not appear to work */

C will let you do a lot of things that make no real sense, as long as they kind of make sense. Crank your compiler warning up high and read them. They have a lot to tell you.

[InterNoctem]

Done rewriting the structures, looks much better. I tried several things to fix my problem. I should note that each time I compile with gcc -o object z_object.c -Wall -Wextra I get:

z_object.c:9:4: warning: "/*" within comment [-Wcomment]
z_object.c:37:4: warning: "/*" within comment [-Wcomment]
z_object.c:43:30: warning: "/*" within comment [-Wcomment]
z_object.c:44:24: warning: "/*" within comment [-Wcomment]
z_object.c:46:32: warning: "/*" within comment [-Wcomment]
z_object.c:48:33: warning: "/*" within comment [-Wcomment]
z_object.c:49:22: warning: "/*" within comment [-Wcomment]
z_object.c:51:40: warning: "/*" within comment [-Wcomment]
z_object.c:55:29: warning: "/*" within comment [-Wcomment]
z_object.c: In function ‘load_zobj’:
z_object.c:125:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
z_object.c: In function ‘main’:
z_object.c:216:14: warning: unused parameter ‘argc’ [-Wunused-parameter]

Leaving everything the same and using

object->dlist->display_data = malloc(size);

(with free() at the end of course) I get the program to run through one loop, increment j then segfault when trying to fread() into object->dlist[j].display_data, I assume this is because malloc allocated space for object->dlist[0] but not object->dlist[1].
The second thing I tried was adding

object->dlist[1].display_data = malloc(size);

which compiles the same but aborts when I try to run it. GDB tells me the problem occurs in the variable buffer but I couldn't find a way to fix it, and I don't see why it only comes up when I attempt to allocate space to dlist[1]. If there are better options for my compiler warnings I'll give that a try. At this point I cant figure out what I'm doing wrong, I'm still learning as I go.

I should also note that I am trying to load buffer with a 32 bit hexadecimal value, so unsigned int* seemed correct.

[vividexstance]

You should post the new code as well.

[InterNoctem]

structures

typedef struct {
	int x;
	int y;
	int z;
	int u;
	int v;
	unsigned int r;
	unsigned int g;
	unsigned int b;
	unsigned int a; 
} vertex;
typedef struct {
	uint8 start;
	uint16 size;
	uint8 half;
	uint8 bank;
	uint32 offset;

	int vertices;

	char* vertex_data;

	vertex vertex[];
} vtx;
typedef struct {
	char* display_data;
	int vtx_calls;

	vtx vtx[];
} dlist;
typedef struct {
	const char* name;
	int display_lists;

	dlist dlist[];
} zobj;

Should I not have made the structure elements arrays e.g. dlist dlist[]?

the problem function:
renamed size to size_file to avoid confusion. The printf lines are for debugging purposes. I should have said before that in main() I declared zobj* object, which is passed into load_zobj().

int load_zobj(const char* filename, zobj* object)
{
	FILE* z_object;
	z_object = fopen(filename, "r");
	if(!z_object)
	{
		fprintf(stderr, "Could not open file.\n");
		return(1);
	}
	object->name = filename;
	long size_file = 0;
	fseek(z_object, 0, SEEK_END);
	size_file = ftell(z_object);
	rewind(z_object);

	/* Parse for display lists. */
	unsigned int* buffer;

	buffer = (unsigned int*)malloc(sizeof(*buffer));

	object->dlist->display_data = malloc(size_file/2);
//	object->dlist[1].display_data = malloc(size_file/2);

	int i = 0;
	int j = 0;
	int k = 0;
	printf("Loading structure.\n");
	while(i < size_file)
	{
		fseek(z_object, i, SEEK_SET);
		fread(buffer, 8, 1, z_object);

		*buffer = ((*buffer & 0xFF000000) >> 24|(*buffer & 0x00FF0000) >> 8|(*buffer & 0x0000FF00) << 8|(*buffer & 0x000000FF) << 24);

		printf("i = %x, and buffer = %08X\n", i, *buffer);
		if(*buffer == 0xE7000000)
		{
			k = 0;
			while(*buffer != 0xDF000000)
			{
				
				printf("i = %x, and buffer = %08X\n", i, *buffer);
				i++;
				fseek(z_object, i, SEEK_SET);
				fread(buffer, 8, 1, z_object);
				fread(&object->dlist[j].display_data[k], 1, 1, z_object);
				printf("%02x\n", object->dlist[j].display_data[k]);
				*buffer = ((*buffer & 0xFF000000) >> 24|(*buffer & 0x00FF0000) >> 8|(*buffer & 0x0000FF00) << 8|(*buffer & 0x000000FF) << 24);
				k++;
			}
			printf("done\n");
			//i += (k + 8);
			j++;
		}
		i++;
	}
	printf("Parsing data.\n");
	object->display_lists = j;
	/* Get vtx structure from display_data. */
	i = 0;
	while(i < object->display_lists)
	{
		j = 0;
		while(j < sizeof(object->dlist[i].display_data))
		{
			k = 0;
			if((object->dlist[i].display_data[j] == 0x01) && (object->dlist[i].display_data[j+4] == 0x06)) /* Check for vtx command and ram bank 06 request. */
			{
				/* Copy data from display list to struct. */
				object->dlist[i].vtx[k].start = object->dlist[i].display_data[j];
				memcpy(&object->dlist[i].vtx[k].size, &object->dlist[i].display_data[j+1], 2);
				object->dlist[i].vtx[k].half = object->dlist[i].display_data[j+3];
				object->dlist[i].vtx[k].bank = object->dlist[i].display_data[j+4];
				memcpy(&object->dlist[i].vtx[k].offset, &object->dlist[i].display_data[j+5], 3);
				k++;
			}
			object->dlist[i].vtx_calls = k;
		} 
		i++;
	}
	printf("Loading vertex data.\n");
	/* load vertex data */
	i = 0;
	while(i < object->display_lists)
	{
		j = 0;
		while(j < object->dlist[i].vtx_calls)
		{
			/* Load vertex_data. */
			fseek(z_object, object->dlist[i].vtx[j].offset, SEEK_SET);
			fread(object->dlist[i].vtx[j].vertex_data, object->dlist[i].vtx[j].size, 1, z_object);
			/* Load vertex[]. */
			k = 0;
			while(k < (object->dlist[i].vtx[j].half/2)) /* Indicates the number of vertices. */
			{
				/* Copy vertex data from offset (k), 16 bytes. */
				memcpy(&object->dlist[i].vtx[j].vertex[k].x, &object->dlist[i].vtx[j].vertex_data + k, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].y, &object->dlist[i].vtx[j].vertex_data + k + 2, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].z, &object->dlist[i].vtx[j].vertex_data + k + 4, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].u, &object->dlist[i].vtx[j].vertex_data + k + 8, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].v, &object->dlist[i].vtx[j].vertex_data + k + 10, 2);
				memcpy(&object->dlist[i].vtx[j].vertex[k].r, &object->dlist[i].vtx[j].vertex_data + k + 12, 1);
				memcpy(&object->dlist[i].vtx[j].vertex[k].g, &object->dlist[i].vtx[j].vertex_data + k + 13, 1);
				memcpy(&object->dlist[i].vtx[j].vertex[k].b, &object->dlist[i].vtx[j].vertex_data + k + 14, 1);
				memcpy(&object->dlist[i].vtx[j].vertex[k].a, &object->dlist[i].vtx[j].vertex_data + k + 15, 1);
				k = k + 16;
			}
		}
		i++;
	}
	printf("Finished.\n");
	free(object->dlist->display_data);
//	free(object->dlist[1].display_data);

	fclose(z_object);
	return(0);
}

[baavgai]

InterNoctem, on 28 August 2013 - 04:04 PM, said:

Should I not have made the structure elements arrays e.g. dlist dlist[]?

Clearly, it's confusing. This is why there is a convention of using Proper case for classes and structures. It's extra confusing because Foo foo[]; doesn't quite mean anything. The compiler is using syntax sugar meant for parameter passing to give you: Foo *foo; .

So..

int load_zobj(const char* filename, zobj *object) { /* we have to assume object is initalized somewhat */
	FILE* z_object;
	long size_file = 0;
	/* ... */
	object->name = filename; /* questionable, might want to allocate at copy the c-string */

	/* This still seems rather pointless
	unsigned int* buffer;
	buffer = (unsigned int*)malloc(sizeof(*buffer));
	
	You could get the same effect with this and no malloc
	*/
	unsigned int buffer;

	/* has dlist be initialized?  when? how?
	why doesn't this barf?
	*/
	object->dlist->display_data = malloc(size_file/2);
	/* if it's uninitualized, then pointer are pointing to, well, anything
	chances are you're trying load an address into a location you don't own
	*/
	/* oops, what is this?  Why would this work if the other one did or vice versa? */
		while(j < sizeof(object->dlist[i].display_data))

[InterNoctem]

Ah yes thank you. I think the problem all along has been that I don't know how to properly initialize struct items, it's a new concept for me. However after rewriting the code to allow buffer as an unsigned int instead of a pointer, I needed to pass &buffer to fread(), which worked fine, but caused the value of i to change, I think i is getting overwritten by buffer. I discovered that declaring buffer after declaring i made the problem stop, but only if I had the line

printf(" compare  %p\n", &i)

before fread(). I'm guessing the problem stems from the use of &buffer, is this because it's a local variable?

[baavgai]

I don't see the code where this happens.

I would recommend breaking up code into functions which work on specific element of your structure. This will give you a better idea of where problems might be, and possibly pointers in general.

e.g.

int copyVertextDataFromOffset(Vertex *vertex, char *vertex_data) {
	int size;
	size = 2; memcpy(vertex->x, vertex_data, size); vertex_data += size;
	size = 2; memcpy(vertex->y, vertex_data, size); vertex_data += size;
	size = 2; memcpy(vertex->z, vertex_data, size); vertex_data += size;
	size = 2; memcpy(vertex->u, vertex_data, size); vertex_data += size;
	size = 2; memcpy(vertex->v, vertex_data, size); vertex_data += size;
	size = 1; memcpy(vertex->r, vertex_data, size); vertex_data += size;
	size = 1; memcpy(vertex->g, vertex_data, size); vertex_data += size;
	size = 1; memcpy(vertex->b, vertex_data, size); vertex_data += size;
	size = 1; memcpy(vertex->a, vertex_data, size); vertex_data += size;
	return size;
}

void loadVertextData(FILE *fh, Vtx *vtx) {
	int k = 0;
	fread(vtx->vertex_data, vtx->size, 1, fh);
			
	while(k < (vtx->half/2)) { /* Indicates the number of vertices. */
		/* note, while k makes sense for an offset vertex_data, 
		I'm note sure if it makes sense for a vertex index
		*/
		k += copyVertextDataFromOffset(vtx->vertex + k, vtx->vertex_data + k);
	}
}

/* guessing from the code above */
void loadVertextDataFix(FILE *fh, Vertex *vertex, int size, int vertexNum) {
	char *p, *buffer = malloc(size);
	int i = 0;
	
	fread(buffer, size, 1, fh);
	p = buffer;
	for(i=0; i<vertexNum; i++) {
		p += copyVertextDataFromOffset(vertex + i, p);
	}
	free(buffer);
}

/* alternately */
void loadVertextDataFix(FILE *fh, Vertex *vertex, int vertexNum) {
	while (vertexNum-- > 0) {
		char buffer[16]; /* knowing that the copy window is 16 */
		fread(buffer, sizeof(buffer), 1, fh);
		copyVertextDataFromOffset(vertex++, buffer);
	}
}

I'll be happy to explain any specific part of this code you might find confusing. However, I strongly recommend reading more on pointers, allocation, and structs. Without a very firm grasp of these fundamentals, you'll just be shooting in the dark.

Hope this helps.