Server Help

[CypherJF]

Okay, I'm having slightly a blonde moment here at 2:40 am...

I wanted to remove the first character if it is the following:

Code: Show/Hide void botInfo::handleGuess(Player *p, char *msg) ...    if (msg[0] == '`' || msg[0] == '-' || msg[0] == ',')    {       char *s = &msg[1];       if (*s == NULL) return;       delete [0]msg;       msg = s;    } ...

The thing is, yes i know it'll modify the msg object in the EVENT call, from there on; which is okay in this case; bad practice i know.. Anyways...

Am I missing something where its 1) memory leaking or 2) would seem to cause a crash? What im mostly concerned about is this line:
if (*s == NULL) return;


ugg. someone help poor cypher whose brain isnt working its best atm lol
_________________
Performance is often the art of cheating carefully. - James Gosling

[50% Packetloss]

im assuming that you allocated memory for whatever *msg is pointing to. Also I have never deleted memory like that, and i dont think you can. Why not just ignore the first character and just do
char *s= msg;

if(blah)
{
s++;
}

i dont like using returns except at the end of functions and never in voids, it will work though

To my knowledge, there are only 2 ways to delete memory. delete blah; and delete [] blah2; , the [] are for arrays

[Mr Ekted]

I assume msg is created with new char[...]?

You can't save a pointer to something, then free it, then use that pointer. This is a perfect example of why I dislike C++. You get used to being able to do "anything" and you get code like this (no offense). The second bug is that changing the value of a parameter does not change the variable that was passed into from the parent function, unless you pass by reference (address).

I can't even make a suggestion for this function. I wouldn't do it like this at all. I wouldn't even have this function exist. In the parent function, I would put the message in a char array (buffer), and use another char * for msg...

Code: Show/Hide void ParentFunc () { char *msg; char  buffer[256]; // put message into buffer msg = buffer; if (*msg == '`' || *msg == '-' || *msg == ',')    msg++; // now use msg as you like }

This function does no allocation, and no freeing.
_________________
4,691 irradiated haggis!

[CypherJF]

thanks XD

[Cyan~Fire]

I don't understand what's so bad about an early return. It only translates into a goto, with less code than using an if statement or something,
_________________
This help is informational only. No representation is made or warranty given as to its content. User assumes all risk of use. Cyan~Fire assumes no responsibility for any loss or delay resulting from such use.
Wise men STILL seek Him.

[50% Packetloss]

They make code confusing to read, a return in the middle of a function translates into a return from subrutine, it pops the program counter+some other stuff off the stack and loads them into thier places. All a goto is, is a memory address to jump to, i suppose it takes the location of where the program starts and adds it to the offset it wants to go to or it takes it's locatio n and adds a number to the program counter to move forward/back.

[Mine GO BOOM]

Here would be my way of doing that:

Code: Show/Hide switch (msg[0]) {     case '`':     case '-':     case ',':         strcpy(msg, msg + 1);         break; }

Even then, the break; is not needed, but it makes the code look cleaner. Plus, this allows you to easily add more 'bad' characters in the future. But do be warned: if your string is not null ended, it will cause a buffer overflow.

[Mr Ekted]

[CypherJF]

But if you didn't fopen and/or want to allocate memory; more or less if the function pre conditions fail, can't you without issue do a premature return?
ie:

int blah(int x) { if (x == 0) return; return 1/x; }

[Jackmn]

Better:

return x ? 1/x : 0;

( Probably with a comment stating what it does, since that is a little ugly )

[-Smong-]

@Cyan~Fire
A goto is a jump and an in most cases an if statement is a subtraction and a conditional jump (ok it's less code, but do you want better readability?).

@50% Packetloss
If you think an early return increases file size because of the stack cleanup, I would imagine a sensible compiler would jump to the end of the subroutine so the cleanup code is shared (may depend on compile flags and the length of the jump).

@Mr Ekted
Why use 'goto Exit' over 'else if'? Should goto be avoided where ever possible (porting to another language could be awkward)?

[Cyan~Fire]

Alright, I see your point.

And why would you ever want to port a C++ program?

[Mr Ekted]

If you use early return in functions where they are ok, you will end up coming back later and adding code to break them. It's just a bad habit. Even though I am in favor of people using their own coding styles for their own files, I refuse to allow early returns for any project that I am in charge of.

If-else works only in very simple cases. My example above uses "blah blah" to show were much more code could break if-else possibilities. There are always cases where you have to use goto, or some crazy crap like while loops with special exit conditions (or breaks), which makes the code even more unreadable.

If you write your stuff in C to begin with, then porting to another language is never an issue, since you started with the most sensible one.

[-Smong-]

I know a couple of bytecode languages that don't support goto, I guess that fits in with 'I am in favor of people using their own coding styles for their own files' as people have preferred languages and target platforms too.

[CypherJF]

Reading the Goto; and return things on MSDN, they said to use break, return, and all other jump statements before using a goto... :/

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccelng/htm/statem_19.asp

I think if I read that right...

[Jackmn]

It's far better to use a goto when cleanup needs to be done prior to the return, which is true for almost everything except small utility functions.

[Mr Ekted]

[CypherJF]

Could be

[Mr Ekted]

Look at some of the run-time and MFC source from VC sometimes. Their code is terrible.

[Mr Ekted]

Here's a function from inside malloc.c. Imagine someone telling you they think there's a bug in it, and to figure it out...

Code: Show/Hide void * __cdecl _heap_alloc_base (size_t size) { #ifdef WINHEAP         void * pvReturn; #else  /* WINHEAP */         _PBLKDESC pdesc;         _PBLKDESC pdesc2; #endif  /* WINHEAP */ #ifdef WINHEAP         if ( __active_heap == __V6_HEAP )         {             if ( size <= __sbh_threshold )             { #ifdef _MT                 _mlock( _HEAP_LOCK );                 __try { #endif  /* _MT */                 pvReturn = __sbh_alloc_block(size); #ifdef _MT                 }                 __finally {                     _munlock( _HEAP_LOCK );                 } #endif  /* _MT */                 if (pvReturn)                     return pvReturn;             }         }         else if ( __active_heap == __V5_HEAP )         {             /* round up to the nearest paragraph */             if ( size )                 size = (size + _OLD_PARASIZE - 1) & ~(_OLD_PARASIZE - 1);             else                 size = _OLD_PARASIZE;             if ( size  <= __old_sbh_threshold ) { #ifdef _MT                 _mlock(_HEAP_LOCK);                 __try { #endif  /* _MT */                 pvReturn = __old_sbh_alloc_block(size >> _OLD_PARASHIFT); #ifdef _MT                 }                 __finally {                     _munlock(_HEAP_LOCK);                 } #endif  /* _MT */                 if ( pvReturn != NULL )                     return pvReturn;             }             return HeapAlloc( _crtheap, 0, size );         }         if (size == 0)             size = 1;         size = (size + BYTES_PER_PARA - 1) & ~(BYTES_PER_PARA - 1);         return HeapAlloc(_crtheap, 0, size); } #else  /* WINHEAP */         /* try to find a big enough free block          */         if ( (pdesc = _heap_search(size)) == NULL )         {             if ( _heap_grow(size) != -1 )             {                 /* try finding a big enough free block again. the                  * success of the call to _heap_grow should guarantee                  * it, but...                  */                 if ( (pdesc = _heap_search(size)) == NULL )                 {                     /* something unexpected, and very bad, has                      * happened. abort!                      */                     _heap_abort();                 }             }             else                 return NULL;         }         /* carve the block into two pieces (if necessary). the first piece          * shall be of the exact requested size, marked inuse and returned to          * the caller. the leftover piece is to be marked free.          */         if ( _BLKSIZE(pdesc) != size ) {             /* split up the block and free the leftover piece back to              * the heap              */             if ( (pdesc2 = _heap_split_block(pdesc, size)) != NULL )                 _SET_FREE(pdesc2);         }         /* mark pdesc inuse          */         _SET_INUSE(pdesc);         /* check proverdesc and reset, if necessary          */         _heap_desc.proverdesc = pdesc->pnextdesc;         return( (void *)((char *)_ADDRESS(pdesc) + _HDRSIZE) ); }

[Cyan~Fire]

I have to admit that the MFC source is better than their CRT source, even though the compiled product is crappier.

And, of course, MFC has the added feature of people not knowing what the heck is actually happening behind the scene. "What, this program's in a loop? Oh."