mysterious seg fault!

I cannot see a problem here:

Code:

vector<bmHit> cur;
uchar byte = getRdata(rf,cur);  /* segfault */
uchar getRdata(FILE *fs, vector<bmHit> &data) {
	uchar byte;
	char buf[128] = {0};
	bmHit cur;
	if (!fscanf(fs, "BYTE %d%*c", (int*)&byte)) return 0;
	fgets(buf,128,fs);
	while (strncmp(buf,"BYTE",4)) {
		sscanf(buf, "%d:%d:%d", &cur.doff,&cur.goff,&cur.len);
		data.push_back(cur);
		fgets(buf,128,fs);
	}
	fseek(fs,-(strlen(buf)),SEEK_CUR);
	return byte;
}

getRdata() completes, the fault is on the return to main -- "Cannot access memory at address" of byte.

Code:

if (!fscanf(fs, "BYTE %d%*c", (int*)&byte)) return 0;

Couldn't lying about the type here have something to do with it?

Originally Posted by anon

Code:

if (!fscanf(fs, "BYTE %d%*c", (int*)&byte)) return 0;

Couldn't lying about the type here have something to do with it?

Yeah, I considered that -- but it doesn't make a difference if I just remove the cast.

However, if I write the value into an int and then copy

byte = x;

it's okay. BUT that is still not necessarily the problem, because just by inserting an extra unused variable (rearranging things), the original runs fine.

It would seem to me that if this:

int x;
uchar b = x;

Does not copy four bytes into b, then:

scanf("%d", &b);

should not either. Am I wrong? I don't want to count on this solution:

Code:

int x;
uchar b;
scanf("%d",&x);
b=x;

if it only works because the problem has been concealed by rearranging the segmentation slightly with extra variables.

> it's okay. BUT that is still not necessarily the problem, because just by inserting an extra unused variable (rearranging things), the original runs fine.
That just makes you lucky, not good.
So you trash some memory that nobody is using "at the moment".

Sooner or later, someone will innocently modify the code in some way, and you're back to wondering why the smoking barrel has suddenly gone off again.

Originally Posted by MK27

Yeah, I considered that -- but it doesn't make a difference if I just remove the cast.

The problem is not the cast to int* (but why are you casting it anyway?), it's the %d.
You are storing an int's worth of data in the space of a char.
My guess is that the crash is from the fscanf overwriting the return address on the stack, so everything seems to be fine in getRdata() until the function ends and you return to some random memory address.

Edit: Missed this part..

It would seem to me that if this:

int x;
uchar b = x;

Does not copy four bytes into b, then:

scanf("%d", &b);

should not either. Am I wrong?

when you do
uchar b = x;
the compiler knows that x won't fit in an uchar and truncates the data.
when you do
scanf("%d", &b); (assuming b is still an uchar)
scanf just sees the memory address of b, it has no type information for what b is. It just assumes that whatever address you pushed on the stack has enough room to store an int.

This is why the C++ `iostream' library is better.

If you only need one byte to be read, only read one byte. You need to use the `%c' specifier. The question is, do you need to read more than one byte?

Soma

In fact _Mike is correct.

Code:

#include <cstring>
#include <iostream>
using namespace std;

int main() {
	unsigned char x[16];
	int i;
	memset(x,27,16);
	scanf("%d",&x[8]);
	for (i=0;i<16;i++) printf("%d\n",x[i]);
	return 0;
}

If you enter 0-255, elements 9-11 are set 0. Obviously enough, casting &x with (int*) does not change this.

Originally Posted by phantomotap

This is why the C++ `iostream' library is better.

So far I am of the opposite opinion, but open to enlightenment -- scan functions seem more succinct and functional than a bunch of >> sequences. Not much difference I guess.

If you only need one byte to be read, only read one byte. You need to use the `%c' specifier. The question is, do you need to read more than one byte?

Yeah. This one is not serialized it is just a plain text file. I'm grabbing the value 101 from, eg: "BYTE 101" and putting it into an unsigned char.

Soma[/QUOTE]

This one is not serialized.

O_o

Maybe you should serialize it; it would then be easy.

I am of the opposite opinion

I know. You are wrong. You can do exactly what you are doing in the same number of lines except it would be type safe; you would have never had this issue.

Soma

Originally Posted by phantomotap

O_o

Maybe you should serialize it; it would then be easy.

Yes, but for now it is quite a bit of data and needs some eyeballing to ascertain some general patterns to decide how it would be best absorbed, so I have to do this the hard way.

I know. You are wrong. You can do exactly what you are doing in the same number of lines except it would be type safe; you would have never had this issue.

I like to be kept on my toes sometimes. Helmets are for clutzy cowards.

Helmets are for clutzy cowards.

O_o

This is why your brains will one day be on the pavement.

Soma

Sorry, I couldn't help laughing at that.

I can't help but wonder if many C programmers would be a lot more inclined to use iostreams if they were just a bunch of calls with regular and non-chained function syntax, rather than using operator overloading.
E.g.

Code:

cout("hello world");
endl();
cout(42);

I mean it all has the same effect underneath.

*sigh*

I hate this forum.

Code:

{don silly armor} This is C++!

When things like this creep up I'm often tempted to "single source" my code that allows the familiar constructs of C in C++ (as Boost::Format). I usually wind up thinking: "They are only using C constructs because they are stubborn.".

Soma

Originally Posted by iMalc

Sorry, I couldn't help laughing at that.

I can't help but wonder if many C programmers would be a lot more inclined to use iostreams if they were just a bunch of calls with regular and non-chained function syntax, rather than using operator overloading.
E.g.

Code:

cout("hello world");
endl();
cout(42);

I mean it all has the same effect underneath.

That would be like Java's print(). Crazy overloading, just not operators.