[mw-devel] CVS Update at silver.sucs.org: marvin

Justin Mitchell arthur at sucs.org
Mon Mar 8 09:59:54 GMT 2004


On Mon, Mar 08, 2004 at 01:28:23AM +0000, Peter Berry wrote:
> 	Added some comments in main_loop() (client and server)
A bit over zealous in places, but okay.

> 	Changed some linked list loops from 'while' to 'for' where this makes more sense
Hate, hate, hate, hate, hate, hate.
I _really_ hate people using for loops for variable length loops,
especially ones involving pointers.
imho for loops should only be used when the length of the loop
is known before the loop begins.
I also think it looks really ugly.

> 	Changed the if-else-if ladder in session_recv() to a switch (client and server)
groan, not a fan of switch statements either, they have a habit of taking
even more space than the if ladder.

> 	Removed a redundant 'else' in session_drop()
cant find this

> 	* What is the purpose of 'int net' in client.c ? It seems redundant to me (the server just uses 'ptr->socket' which is currently always the same as 'net' in client.c).
Its less typing and makes the code a little easier to read rather than having
a structure lookup repeatedly.

> 	* Can we really only do one I/O operation on each socket for each call to select() ? I.e. can the 'else's be removed ?
Only one of each type thats flagged as ready,
you can reorganise that if/else ladder, the read and write arent really 
exclusive but the error state is.
oh and i suspect that the session_drop(ptr) on line 90 should have been 'old'

> 	* The client currently has debug mode turned off; is there a reason for this ? (also '#define DEBUG' and #ifdefs might be more sensible than a variable)
Theres not that much code in there yet, give it time to fill out.

> 	Possible bugs:
> 	* Sometimes it seems the server doesn't actually receive any data
>  - e.g. size=0 expect=5 . In fact it seems only to receive the first message
> intact. But the quit command always works, so apaprently it really does
>  receive it.
Your not guaranteed to receive anything intact, thats why theres all
that buffering and chopping up stuff in session.

ps. please wrap your messages, or set your mua to do it for you,
its a real pita to have to do it manually when replying.



More information about the mw-devel mailing list