tags:

views:

345

answers:

8

Hello,

All I need this for is strcpy().

I want to see whether the first three bytes of a buffer(byte array) are "JMX" as string.

This is what I did so far:

char * ddj;
strcpy( ddj, buffer ); //buffer is BYTE[]
if ( strcmp( "JMX", ddj ) == 0 ) //check first three chars are "JMX"
{
    buffer += 20;   //increase the index with 20
    size -= 20;     //int
}

I get exception at strcmp() line. What is the problem?

I wish I was writing this in C# :(

+2  A: 

You have not allocated memory for ddj. Allocate memory using new to it . For example

char *ddj = new char[size]; //Allocate size number of chars
//do the required comaprisons

delete[] ddj; //Remember to release the memory.

On the other hand you can use std::string which is a standard string class.

Naveen
A: 

This is UB because ddj isn't pointing to anything. You need to allocate memory:

char* ddj = new char[strlen(buffer) + 1];

Be sure to delete the memory you allocated using delete[] (not plain delete!).

You could also use std::string which is generally safe, as you don't have to deal with pointers and memory alloation.

Looking at your code, however, ddj seems useless. Just use buffer:

if ( strcmp( "JMX", buffer ) == 0 ) //check first three chars are "JMX"
{
    buffer += 20;   //increase the index with 20
    size -= 20;     //int
}
strager
strlen(buffer) + 1
anon
@Neil, Thanks for the correction.
strager
+8  A: 

Tho things go wrong here:

  1. ddj does not point to any actual memory. Hence the copy will have undefined behavior
  2. The copying is not necessary in the first place.

This is what you can do:

if(strncmp("JMX", buffer, 3) == 0) {
  buffer += 20;
  size -= 20;
}

This uses strncmp instead of strcmp, thus ensuring that no more than three bytes are compared. If buffer can contain less than three bytes, you should do something like:

if(buf_len >= 3 && strncmp("JMX", buffer, 3) == 0) {
  buffer += 20;
  size -= 20;
}
Stephan202
+2  A: 

You must allocate new memory for ddj. Either declare it as

char ddj[NAX_LENGTH];

or with dynamic allocation

char* ddj = new char[length]; // You must use delete[] to free the memory in the end.

A more convenient alternative is std::string.

Dario
A: 

if you want to strcmp ddj, you can also do it on buffer first, and make a copy of buffer if you need it later.

動靜能量
+6  A: 

You're not allocating any memory for ddj. Since it's a local variable, it's allocated on the stack. Local variables are not initialized to 0/false/NULL by default, so the value of ddj immediately after it's declared is undefined -- it will have the value of whatever is left in memory at that particular location on the stack. Any attempt to dereference it (that is, to read or write the memory at which it's pointing) will have undefined behavior. In your case, it's crashing because it's pointing to an invalid address.

To fix the problem, you need to allocate storage for ddj. You can either allocate static storage on the stack, or dynamic storage on the heap. To allocate static storage, do:

// Allocate 64 bytes for ddj.  It will automatically be deallocated when the function
// returns.  Be careful of buffer overflows!
char ddj[64];

To allocate dynamic storage:

// Allocate 64 bytes for ddj.  It will NOT be automatically deallocated -- you must
// explicitly deallocate it yourself at some point in the future when you're done
// with it.  Be careful of buffer overflows!
char *ddj = new char[64];
...
delete [] ddj;  // Deallocate it

Instead of managing storage yourself, it would be a better idea to use std::string, which automatically deals with memory management.

Finally, since all you're doing is comparing the first three characters of the string, there's no need to jump through hoop to copy the string and compare it. Just use strncmp():

if(strncmp(buffer, "JMX", 3) == 0)
{
    ...
}
Adam Rosenfield
+1  A: 

Firstly, it's crashing because ddj doesn't point to anything.

Secondly, you don't need to copy the data from byte[] to char* (they're essentially the same thing). You can just do:

if (strncmp("JMX", reinterpret_cast<char*>(buffer), 3) == 0)
{
  // Strings are equal, do what you want
}
Mark Ingram
For builtin types, "static_cast" will only work on pointers where the implicit conversion will work anyway. You should probably change this to "reinterpret_cast".
Richard Corden
Yes, good spot. Thanks
Mark Ingram
A: 

You are getting the exception because the variable 'ddj' isn't initialized. It is pointing at garbage, so who knows where you are copying that string...

You don't really need to copy the bytes before comparing them, though.

if(strncmp("JMX", buffer, 3) == 0) // check if the first three characters are "JMX"
{
    buffer += 20;
    size -= 20;
}
Corey Ross