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

[Peter Berry]

On Mon, 8 Mar 2004, Justin Mitchell wrote:

> On Mon, Mar 08, 2004 at 01:28:23AM +0000, Peter Berry wrote:
> > 	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.

As far as I can see, that's really the purpose of the C for statement, 
i.e. so you can do for any linear structure the equivalent of 'for 
(i=0;i<n;i++)'. I don't see why this is a problem where the loop length is 
variable as long as your code is sane.

Obviously ugliness is a matter of opinion. Personally I'd rather have the 
convenience of continue than have to use goto or do everything manually 
every time.

Anyway, I didn't change all of them... some really are ugly in the for 
form and one of them doesn't actually work that way.
 
> > 	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.

fair enough, but they are a lot easier to read, hence the change.
 
> > 	Removed a redundant 'else' in session_drop()
> cant find this

doh, never committed the change. Redone.
 
> > 	* 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.

OK, why not use it in the server too?
 
> > 	* 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'

Done.
 
> > 	* 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.

OK...
 
> > 	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.

The weird thing is that the server apparently receives the data and yet 
'size' is always 0 except for the first message.
 
> 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.

will do.

-- 
Peter Berry
Treasurer, Swansea University Computer Society
<pwb at sucs.org>