Server Help

Trash Talk - ostrstream

Cyan~Fire - Tue Jun 15, 2004 1:30 pm
Post subject: ostrstream

(Ekted please ignore this C++ help post.)

I've been trying to write to memory using ostrstream's dynamic buffer feature. However, it seems that whenever I write a bunch of nulls stuff gets totally whacked up. Here's the code that I'm using:

Code: Show/Hide

ostrstream tout();
tout.write((char*)&next_uid, 4);
num = 0xCF9328F6;
tout.write((char*)&num, 4);
for (i = 0; i < NUM_PLAYERS; i++)
{
len = strlen(players[i].name);
tout.write(players[i].name, len); //Access violation here on second run-through
NULLS(0x100 - len);
}

On delving into debug information, it appears that the nulls were written correctly, and the put pointer advanced correctly, but the end pointer unchanged. However, it does seem that the memory was allocated since there is 4 bytes of no-man's-land after the nulls. Then, when the problemating instruction allocates the memory for the 2nd name, the put pointer ends up at some ridiculously small memory address, obviously not in the debug heap.

Help?

Mr Ekted - Tue Jun 15, 2004 1:53 pm
Post subject:

(haha)

players[i].name is not always going to be valid right? You need to test for null. I don't know MERV. Is player[i] always valid?

Cyan~Fire - Tue Jun 15, 2004 8:17 pm
Post subject:

This ain't MERV, it's an app that I'm writing for a completely different game.

I've already checked for nulls of course, on entry to ostrstream::write(), the char * parameter has the correct name inside.

Maucaub - Tue Jun 15, 2004 9:23 pm
Post subject:

what is NULLS() doing? (not conceptually, I already get that ... what's the actual code)

Mr Ekted - Tue Jun 15, 2004 10:29 pm
Post subject:

Add this just before the call to tout.write(players[i].name, len):

printf("%08x\n", players[i].name);

See if the value looks "normal" or is very small. Btw, can I see your players struct/class? I hope name is a char * and not a string type.

Cyan~Fire - Tue Jun 15, 2004 11:47 pm
Post subject:

Sorry, that was stupid of me not to include my macro.

Code: Show/Hide

#define NULLS(size) \
tout.seekp(size, ios::cur);

That macro works just fine for an ofstream...
I guess whoever wrote this section of the CRT didn't implement seeking well.

Code: Show/Hide

class Player
{
public:
Player();
~Player();
void reset();

char name[30];
int stable;
int enable, human, civ;

char ai[_MAX_FNAME];
char *aifile; //mmapping of AI file for exporting

unsigned long resources[4]; //gold, wood, food, stone

int dis_tech[0x1E], dis_unit[0x1E], dis_bldg[0x14];
int ndis_t, ndis_u, ndis_b;

float unknown[6];
float pop;
float camera[2];
short u1, u2;
bool avictory;
char diplomacy[9];
int color;

SVector <Unit> units;
};

And I can't exactly printf(), since it's a Windows UI app, but I'll try to implement some message box thingy. That's gonna be annoying seeing as the file my code is in doesn't have windows.h included. Grrr.

Mine GO BOOM - Wed Jun 16, 2004 12:17 am
Post subject:

Mr Ekted wrote:

Add this just before the call to tout.write(players[i].name, len):

printf("%08x\n", players[i].name);

If you are using a UI, you could always do it this way:

Code: Show/Hide

FILE *fDebug = fopen("debug.log", "w+");
if (fDebug)
{
fprintf(fDebug, "%08x\n", players[i].name);
fclose(fDebug);
}

If you want to this a bunch of times, I'd recommend changing the "w" part to "a+", which means append, instead of creating a new blank file.

Or you could learn to use a debugger and set breakpoints. Those are always handy. What compiler/debugger set you using? If its MSVC 5 or 6, I can help teach you how to play with the debugger.

Dustpuppy - Wed Jun 16, 2004 10:27 am
Post subject:

I've never used ostrstream before, I was looking around for info on it and it seemed the general consensus was that it was deprecated, and you should use ostringstream instead.

Cyan~Fire - Wed Jun 16, 2004 1:35 pm
Post subject:

I've been looking through all the debug functions, followed all the CRT functions and what they're doing. I still can't quite understand what's going on but I'm gonna run through it a few more times and see what's happening. But as I said earlier, my data is not bad, because inside the function, I can see the correct name.

Dust wrote:

I've never used ostrstream before, I was looking around for info on it and it seemed the general consensus was that it was deprecated, and you should use ostringstream instead.

Yeah, I've seen that before, but sadly ostringstream uses std::strings, which are a part of STL, which I'm not using for reasons Ekted would appreciate

Mr Ekted - Wed Jun 16, 2004 2:00 pm
Post subject:

Seems my lasy reply disappeared. I'll try to recreated it.

Please post your players struct/class. I'm guessing the name member is a string type and not a char *, which would be a huge bug.

Cyan~Fire - Wed Jun 16, 2004 2:18 pm
Post subject:

OK, after a lot of debugging, I think I found where the error is occuring.

Code: Show/Hide

streampos strstreambuf::seekoff(streamoff off, ios::seek_dir dir, int mode)
{
char * tptr;
long offset = EOF; // default return value
//cut out ios::in funcs
if (mode & ios::out)
{
if (!epptr())
{
if (strstreambuf::overflow(EOF)==EOF)
return EOF;
}
switch (dir) {
case ios::beg :
tptr = pbase();
break;
case ios::cur :
tptr = pptr();
break;
case ios::end :
tptr = epptr();
break;
default:
return EOF;
}
tptr += off;
offset = tptr - pbase();
if (tptr < pbase())
return EOF;
if (tptr > epptr())
{
if (x_dynamic)
{
x_bufmin = __max(x_bufmin, (tptr-pbase()));
if (strstreambuf::doallocate()==EOF)
return EOF;
}
else
return EOF;
}
pbump(tptr-pptr());
}
return offset;
}

OK, as you can see tptr is where the put pointer should be after the seek--the target pointer. It's fine before allocation, but after allocation, the put pointer is now pointing to the end of the already written data in the NEW block of memory, while the target pointer is pointing to where the put pointer should be in the OLD block of memory. Hence, the put pointer is bumped to a section in the old block, and you can imagine where it goes from there.

Oops Microsoft, your runtime library is screwed.

Mr Ekted - Wed Jun 16, 2004 2:53 pm
Post subject:

I'm skeptical that it's a bug. Don't you think this would have been tested at all? Also, reply to my previous post.

Mr Ekted - Wed Jun 16, 2004 3:06 pm
Post subject:

Cyan~Fire wrote:

Yeah, I've seen that before, but sadly ostringstream uses std::strings, which are a part of STL, which I'm not using for reasons Ekted would appreciate

You still are using some pre-built class to do your work for you. See what happens when you do that? You don't understand what's going on inside, and you get into trouble.

Cyan~Fire - Wed Jun 16, 2004 4:35 pm
Post subject:

I already posted my class.

And I do understand (now, at least), what's going on behind the scenes. I'm using an iostream because it's well-documented, and has a pretty good interface, IMO. At some point one does have to resort to pre-written code, you demonstrated that yourself when you suggested using printf() for debugging.

Mr Ekted - Wed Jun 16, 2004 5:44 pm
Post subject:

You really should learn to interactively debug. The problem you are having would be obvious in about 60 seconds.

Cyan~Fire - Wed Jun 16, 2004 7:03 pm
Post subject:

I really dont know what I'm doing wrong... could you perhaps clue me in?

I tried replacing the ostrstream with an ofstream and it worked exactly how it should. The iostreams are all supposed to have the same interface, so I really don't understand how I could be doing anything wrong.

Mr Ekted - Wed Jun 16, 2004 7:54 pm
Post subject:

I don't know how either work. I don't use pre-built classes for anything. It takes only about 10% more work to to it yourself, and there's no overhead. Look at some of the functions sometimes. You call a class method, it calls 7 other class methods deep, and ends up calling malloc(). I just call malloc(). I understand the theory of re-useable code through wrapping functionality, but in practice, I have never found it to be worth the trouble.

Cyan~Fire - Wed Jun 16, 2004 8:32 pm
Post subject:

Wait... I thought you know what I was doing wrong... or you were just telling me I could easily find out? I'm confused now.

Anyway, the only reason I'm using wrappers here is for the "dynamic buffer" capability, which doesn't seem to be doing it's job. Believe me, I'd rather use a FILE any day, I love formatted printing.

So if Ekted doesn't know what I'm doing wrong... does anybody else?!

Mr Ekted - Wed Jun 16, 2004 10:33 pm
Post subject:

My suggestion to Cyan, after talking to him about this issue:

Instead of serializing the data into a stream class, which involves lots of function calls into unknown (and seemingly very bad) code, just create a packed structure with the exact format you want and fill it in...

Code: Show/Hide

#pragma pack(1)

struct MyData
{
unsigned long next_uid;
unsigned long num;
char name[NUM_PLAYERS][256];

// etc.
};

#pragma Pack()

function ()
{
int i;
MyData data;

memset(&data, 0, sizeof(data));
data.next_uid = next_uid;
data.num = 0xCF9328F6;

for (i = 0; i < NUM_PLAYERS; i++)
strcpy(data.name[i], players[i].name);

// blah blah blah
}

Maucaub - Sat Jun 19, 2004 2:40 am
Post subject:

I guess this issues has effectively been resolved since my last visit, but I'll add a couple more cents anyway, should it prove valuable to someone.

First, it would seem I share a number of the same opinions and philosophies as Mr.Ekted ... this is NOT how I would have handled this problem. However, if you're truly hard-up to continue along these lines and/or this is useful for future reference, here are a few other pointers.

* As Dustpuppy already mentioned, ostrstream IS deprecated by Standard C++. This implies that implementation is no longer guaranteed and would suggest not using it for new code.

* If you're working with strings, treat them as strings. If working with binary data, treat it as binary data. Your use of write() to write the name is inconsistent with this approach and is sure to cause nothing but headaches one day. The write() function treats the buffer as binary data, hence there is NO null terminator added to the "string". Sure, in this case your intention is to fill the remaining space with 0's, but in general this is asking for trouble. If you truly want to treat it as a string, you can simply use the output operator (<<).

* I would not rely on seekp() for this for two reasons: 1) the reallocation implementation is not as clear when you're not actually writing to the dynamic array, and 2) even if the reallocation occurs properly, the initialization of the memory is unclear. You might get the memory you need, but assuming it will be filled with NULLs is shaky ground. If you want a buffer filled with 0's, put them there yourself. Also, here's an example of where write() would end up causing you trouble. If you don't actually get a buffer properly padded with zeros, your string will not end up null terminated, and something will go wrong. Had you written the string as a string and this happened, the program would never know since the strings are all properly terminated.

* When working with a fixed size record, you'll save yourself these kinds of headaches, as well as improve performance, simply by treating them as fixed sized records. Operate on it as a single unit. When you're writing binary data, it doesn't matter how long the string inside is. Just write the whole buffer. For example, modifying your own code:

Code: Show/Hide

char buff[256];
ostrstream tout();
tout.write((char*)&next_uid, 4);
num = 0xCF9328F6;
tout.write((char*)&num, 4);
for (i = 0; i < NUM_PLAYERS; i++)
{
memset( buff, 0, sizeof(buff) );
strncpy( buff, players[i].name, sizeof(buff) );
tout.write( buff, sizeof(buff) );
}

This is merely a kludge, but it illustrates the point. In this approach, write() handles the reallocation, a fixed size binary buffer is written each time, and the string contained in the buffer is always padded with zeros up to the buffer size.

* Lastly, if you KNOW how big NUM_PLAYERS is, and you KNOW that you want each player's name to be stored in a 256-byte buffer, there's no need for a dynamically reallocated/resized array. Either create a static packed array (as in Ekted's example), or if the buffer needs to be dynamic, allocate your known size in one fell swoop instead of reallocating and re-free'ing little chunks over and over.

Hope these tips help, if not for this, perhaps for something else.

Cyan~Fire - Sat Jun 19, 2004 12:37 pm
Post subject:

I agree with almost all of your statements, and thanks for your help

. However, there are a few instances where your suggestions do not apply to what I'm doing (even though you probably couldn't have known this).

Macaub wrote:

* If you're working with strings, treat them as strings. If working with binary data, treat it as binary data. Your use of write() to write the name is inconsistent with this approach and is sure to cause nothing but headaches one day. The write() function treats the buffer as binary data, hence there is NO null terminator added to the "string". Sure, in this case your intention is to fill the remaining space with 0's, but in general this is asking for trouble. If you truly want to treat it as a string, you can simply use the output operator (<<).

This would be true if I were just writing a nice, pretty text file, but I'm not. As you can see by my direct writing of num, I'm working with a binary stream here which will be compressed by zlib in the near future. So, I see no reason to mix formatted and unformatted output, when the output really is unformatted. Also, most strings in the file do not rely on null termination, rather a 2-byte length value preceding them.

I definitely do agree with your suggestion to make a buffer for the players' names, and it makes me wonder why I didn't think of this in the first place.

Macaub wrote:

* Lastly, if you KNOW how big NUM_PLAYERS is, and you KNOW that you want each player's name to be stored in a 256-byte buffer, there's no need for a dynamically reallocated/resized array. Either create a static packed array (as in Ekted's example), or if the buffer needs to be dynamic, allocate your known size in one fell swoop instead of reallocating and re-free'ing little chunks over and over.

I'd love to do it this way, but it really wouldn't end up pretty. The section of code I posted here is just that- a section of code. I'm writing a whole bunch of variable length sections after this, and calculating the size of each before hand would mean a LOT of very messy code and possible errors. I know that the code behind ostrstream is messy anyway, but at least this way it didn't look so on the surface.

However, since I prefer FILE input/output instead of the fstreams, and that I will use Ekted's method for clumping the data beforehand in the future, I will very likely never use iostreams again. The point is moot.