[C++] Inserting Null-Terminating Strings. (Cont.)

[Arta]

The latter method is the one I use.

[Sorc.Polgara]

And here is a more elegant way (with a few redundancies which you'll have to get over for the sake of elegancy), which you can think about while writing your packet buffer:

Code Select Expand


int pos = 0;

*(WORD*)(buff + pos) = pSize;
pos += sizeof(WORD);

*(BYTE*)(buff + pos) = pID;
pos += sizeof(BYTE);

*(DWORD*)(buff + pos) = prodID;
pos += sizeof(DWORD);


Yeah, I like this one better for some reason.  Perhaps it is b/c it is easier to look at or hmmm I don't know how to put it, but it explains more I guess.

I put in both your ones with into my debug function and it gave the same results that mine did.  So my mine must work.  However I suppose it is better to not use memcpy() when you don't need to?

The thing about C++ and VB6 that that I find significant is the use of pointers and stuff.  In VB I had to use strings for mostly everything when I handled packets.  In C++ you can make use of different kind of datatypes when handling packets.

k thanks alot.  I'll go make my packet buffer ^_^;  I think I also fixed my DebugOutput function too.

[Sorc.Polgara]

Dangit!

My packet buffer, what I've done so far, uses a fixed-size buffer i.e. "char buf[7]".  I would sort of like to use a dynamic array.  I've experimented with the code "char *dynArray = new char[mysize]" along with the "delete [] dynArray" that destroys it or deallocates it from memory.

Also, inserting a null-terminating string into the non-dynamic buffer could be done using the memcpy() and/or strcat() functions?

Another question, is there any difference between using the datatype LPSTR and char*?  Is memmove() prefered over memcpy() in such operations?



EDIT:  Sorry for the double post.  I felt that it was appropriate enough.

[Adron]

What depends on a fixed size?

You may have to reallocate your buffer at times. Whenever it's full. One common method is to double the size each time you reallocate.

Inserting a null-terminated string could be done using strcpy, memcpy or memmove. You need to make sure it will fit before you insert it.

LPSTR and char* are the same in current Windows.

You have to use memmove if the source and destination buffers may overlap. I don't think memmove will be optimized the way memcpy can be.

[Sorc.Polgara]

What depends on a fixed size?

My packet buffer inserts into a string/array already set to a specific size.

You may have to reallocate your buffer at times. Whenever it's full. One common method is to double the size each time you reallocate.

Reallocating, is that where the "char *dynArray = new char[changesize];" kicks in?  When I reallocate, is it possible to preserve the data already in the buffer?  I've tried using the "char *dynArray = new char[changesize];" but it loses the data I've already stored in it.

[Kp]

To preserve the data, the old data must be copied to the new buffer.  This is done automatically if you use realloc(3) or HeapReAlloc.  However, C++ lacks a realloc operator, so you must instead create a new array, explicitly copy the data, and then explicitly release the old array (all this would be handled for you by realloc).  Also note that create/copy/free may fail (or waste effort) in cases where realloc succeeds if the only suitable place for the new allocation is growing of the old allocation into free space which follows it.

[Skywing]

Yoni's method may fail on IA64 due to alignment faults.

[Sorc.Polgara]

Ok, my packet buffer seems like it works, but it doesn't =\


This is the class that is located in the header file "packetbuffer.h"

Code Select Expand


// packetbuffer.h
// packet buffer class

#ifndef PACKETBUFFER_H
#define PACKETBUFFER_H

class pbuffer{
public:
pbuffer();
pbuffer(size_t);
~pbuffer();
void InsertDWORD(DWORD);
void InsertWORD(WORD);
void InsertBYTE(BYTE);
        int resizeBuf(size_t);

private:
size_t bufsize;
int pos;
int flag;
char* buffer;
};
#endif


Ok I'm only going to show the member functions InsertDWORD(DWORD), the resizeBuf(size_t), and pbuffer() and pbuffer(size_t) because these are the only ones that are used or executed when testing the packet buffer.  They are all located beneath the #endif of course.

Code Select Expand


// constructor pbuffer()
pbuffer::pbuffer()
{
pos = 0;
bufsize = 0;
flag = 0;
}

// constructor pbuffer(size_t)
pbuffer::pbuffer(size_t bsize)
{
pos = 0;
flag = 1;

if((bsize <= 0) || (bsize == NULL))
{
if( (buffer = (char*)malloc(sizeof(char*))) == NULL ){
printf("Error allocating buffer sizeof(char*)\n");
exit( 1 );
}
}
else
{
if( (buffer = (char*)malloc(bsize)) == NULL ){
printf("Error allocating the buffer sizeof(bsize)\n");
exit( 1 );
}
}
bufsize = _msize(buffer);

cout <<"Allocated buffer size: " << bufsize << endl << buffer << endl;
}

// member void InsertDWORD(DWORD)
void pbuffer::InsertDWORD(DWORD insDWORD)
{
if( resizeBuf(sizeof(DWORD)) == false ){
printf("Error resizing the buffer to insert a DWORD");
exit( 1 );
}
else{
cout << "Successfully inserted: " << sizeof(buffer) << endl;
}
*(DWORD*)(buffer) = insDWORD;
cout << buffer << endl;
}

// member int resizeBuf(size_t)
int pbuffer::resizeBuf( size_t resize )
{
// buffer is not allocated yet
if( (bufsize) <= 0 || (bufsize == NULL) ){
if( (buffer = (char*)malloc(resize)) == NULL ){
cout << "Error allocating the buffer: " << resize << endl;
return false;
}
cout << "Successfully allocated the buffer: " << sizeof(buffer) << endl;
}
else{
if( (buffer = (char*)realloc(buffer, (bufsize + resize))) == NULL ){
cout << "Error resizing the buffer: " << resize << endl;
return false;
}
cout << "Successfully resized the buffer: " << sizeof(buffer) << endl;
}
return true;
}


This is my main() function

Code Select Expand


#include <iostream.h>
#include <malloc.h>
#include <stdlib.h>
#include "packetbuffer.h"

int main(void)
{
DWORD prodID = 0x07;

pbuffer pbuf(sizeof(DWORD));
pbuf.InsertDWORD(prodID);

return 0;
}


It compiles fine, but when I run it I get

Allocated buffer size: 4
====²²²²
Successfully resized the buffer: 0x0042F0704
Successfully inserted: 4
*beep*

the *beep* is brief beeping noise comes directly out from my desktop tower, the build in speaker that is hooked up to my mobo.

Everything seems to be going fine based on the stuff I put there to tell me what is happening.   Except for that beep...

It looks as if the beeping occurs when it hits the end of InsertDWORD(DWORD) where the "*(DWORD*)(buffer) = insDWORD;" is.  It should work, its basically the same thing that Yoni posted and had worked when I put it in the main() function...

dunno, help prz.

[Adron]

Outputting a 0x07 is an instruction to the computer to beep. So it's doing exactly what you told it to do.

Some comments on your code:

I don't see why you allocate an initial buffer of 4 bytes and then immediate resize it to add 4 more bytes, when you've only added a single 4 byte DWORD to it.

You shouldn't be using sizeof on buffer, because that won't change as you resize the buffer. Track the allocated size in your own variable bufsize.

Don't resize the buffer every time you add data. Resize it in larger increases.

[MyndFyre]

I noticed in your resize function:

Code Select Expand


if( (bufsize) <= 0 || (bufsize == NULL) ){


You probably want to check if buffer, not bufsize, is NULL.  Rewritten:

Code Select Expand


if ( (bufsize <= 0) || (buffer == NULL) ) {


[Mephisto]

By first look of your InsertDWORD member function you're not really creating a packet buffer...

Examine this statement:

Code Select Expand

*(DWORD*)(buffer) = insDWORD;
You're not actually inserting a DWORD, you're just reassigning your buffer to that value.  You should use something like this in your function:

Code Select Expand

if ((CurrentBufferLength + sizeof(DWORD) > MaximumBufferSize))
return;
memcpy(&buffer[CurrentBufferLength], &insDWORD, sizeof(DWORD));
CurrentBufferLength += sizeof(DWORD);

[Adron]

Or,

Code Select Expand


*(DWORD*)(buffer+pos) = insDWORD; pos += sizeof(DWORD);


[OnlyMeat]

Or,

Code Select Expand


*(DWORD*)(buffer+pos) = insDWORD; pos += sizeof(DWORD);


Im sorry but yuk thats ugly i think for purposes of readabillity alone they should atleast be on 2 lines:-

Code Select Expand


memcpy( buffer+pos, &insDWORD, sizeof(DWORD) );
pos+= sizeof(DWORD);


On the other point i have to agree with adron reallocating every time you add a new dword is very inefficient and will cause fragmentation of the heap. If you are going to use a dynamic sizing buffer then try and make a smart one that does a minimum allocation of say 32 bytes or more so you can atleast add 8 dwords before it has to resize.

[Eibro]

It seems a vector or even a deque would work well here...
We are using C++, right?

[Adron]

Code Select Expand

(-1)[(DWORD*)(buffer + (pos += 4))] = insDWORD;