Public Group

Access violations

This topic is 3975 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

Recommended Posts

char* convert(const unsigned char* name) {
char buf[512], *tt = NULL, *p = NULL;
int i, val;

tt = buf;
while( 1 ) {
val = *name++;
if( val == 0 )
break;
for(i=0; i<val; i++) {
*tt++ = *name++;
}
*tt++ = '.';
}

*(tt-1) = 0;

p = (char*)malloc(strlen(buf)+1);
strcpy(p, buf);
return p;
}

unsigned char* decompress(unsigned short len, unsigned char* data, unsigned char* buf) {
unsigned char temp[512];
int j, offset=0;
for(j=0; j<len; j++) {
if( (data[j] >> 4 & 0xc) == 0xc ) {
unsigned char* ins = &buf[data[j+1]];
memcpy(temp+j, &buf[data[j+1]], strlen((char*)ins));
offset += strlen((char*)ins);
j++;
}
else
temp[j+offset] = data[j];
}
temp[j+offset] = 0;
return temp;
}

...
struct RR {
unsigned char* name;
}
char inBuf[65536];
RR rr;
rr.name = &inBuf;
unsigned char * pop = decompress(strlen((char*)rr.name), rr.name, inBuf); char* out = convert(pop); In the above code, when I call decompress, it stores the correct value in pop. However when I call convert(pop), I get an access violation, because the value of the input parameter to convert is all garbled, instead of the value of pop.

Share on other sites
Quote:
 Original post by Sfpianounsigned char* decompress(unsigned short len, unsigned char* data, unsigned char* buf) { unsigned char temp[512];//.... return temp;}

You're returning a local variable. After calling decompress, temp becomes invalid. You may be able to still use it, but surely after calling another function you'll get an access violation.
Too strange that your compiler didn't warn you about that
You should change:
unsigned char temp[512];
for
unsigned char *temp = new char[512];
(or it's C counter part, unsigned char *temp = (char*)malloc( 512*sizeof(char) );

Note that after calling convert(), this is, after finishing using pop; you should destroy memory:
delete [] pop;
(or it's C counter part, free(pop); )

You may also consider if your app is not very complex, to make pop a global variable (not recommended)

If you're using C++; you may consider using smart-pointers from the STD library. Learn about them, they will free you from the ugly work of dealing with memory management.

Hope this helps
Dark Sylinc

Share on other sites
Quote:
Original post by Matias Goldberg
Quote:
 Original post by Sfpianounsigned char* decompress(unsigned short len, unsigned char* data, unsigned char* buf) { unsigned char temp[512];//.... return temp;}

You're returning a local variable. After calling decompress, temp becomes invalid. You may be able to still use it, but surely after calling another function you'll get an access violation.
Too strange that your compiler didn't warn you about that
You should change:
unsigned char temp[512];
for
unsigned char *temp = new char[512];
(or it's C counter part, unsigned char *temp = (char*)malloc( 512*sizeof(char) );

Note that after calling convert(), this is, after finishing using pop; you should destroy memory:
delete [] pop;
(or it's C counter part, free(pop); )

You may also consider if your app is not very complex, to make pop a global variable (not recommended)

If you're using C++; you may consider using smart-pointers from the STD library. Learn about them, they will free you from the ugly work of dealing with memory management.

Hope this helps
Dark Sylinc

Uh, in C++ we represent strings with std::string and avoid all that nonsense. The only smart pointer in the *standard* library is std::auto_ptr, and although they are useful here, they're a very roundabout way of addressing the problem (by insisting on still thinking about the data at too low a level).

I tried to start cleaning up the code, but got stuck, because I can't figure out what you're really trying to do. It seems like "convert" is intended to replace length-counted substrings with period-terminated ones, but I can't make any sense out of the "decompress" algorithm at all. (In particular, you memcpy() to 'temp + j', but then increment j by 1 to prepare for the next write - isn't that going to scribble over the second and subsequent characters that were copied?)

Share on other sites
Quote:
 Original post by ZahlmanUh, in C++ we represent strings with std::string and avoid all that nonsense.

Believe me, I would if I could.

Quote:
 You should change:unsigned char temp[512];

I though that might be the problem, but I was trying to avoid having a bunch of malloc'd things to keep track of. Thanks guys.

Share on other sites
Quote:
Original post by Sfpiano
Quote:
 Original post by ZahlmanUh, in C++ we represent strings with std::string and avoid all that nonsense.

Believe me, I would if I could.

Why is it that you can't? Also, convert is unsafe (it has a potential buffer overrun). I suspect it's not the only thing that is, but it's all I've really looked at so far.

Share on other sites
Btw, the VS compiler should give you a warning for "returning address of temporary" which would have pointed you at the problem quicker. If you're using VS you might want to see if you can crank up the warning level to something higher.

(If you're using gcc, then ignore this).

1. 1
2. 2
JoeJ
20
3. 3
4. 4
frob
13
5. 5

• 13
• 18
• 13
• 20
• 13
• Forum Statistics

• Total Topics
632194
• Total Posts
3004681

×