On Tue, 7 Feb 2012, Alexander Lamaison wrote:
Thanks a lot. I think we're doing good progress!
> - should 'already' be caulculated later as the processing of leftovers
> can change the offset it is based on
It can be moved down a bit into the switch() since the variable is only used
in the 'idle' state. If we make the leftover code return immediately (as the
question/answer say) there won't be any need to calculate 'already' after it.
I agree with your point that it would otherwise have to.
> - why do we not return immediately when we've processed leftovers; as
> Peter pointed out, we aren't allowed to anything that could access the
> transport layer anyway in that case so why carry on?
You mean in the leftover processing first in the idle case? Yes, that can
probably return at once to make the code clearer.
> - why does the second 'phase' set the next state as state_idle rather
> than state_sent2 (not that it's actually used but it documents intent)
It sets the state to idle to make sure it doesn't wrongly get stuck in the
sent2 state. It sets the state2 only when an error (like EAGAIN) is received.
> - is total_read guaranteed to be > 0 at the end of the function (where it
> previously MOOed)
No, it can also be zero in case all this function read was meta-data that
wasn't actually payload and this function should be OK to have 0 returned for
that case.
> - why check total_read in the second phase while-loop? the loop doesn't
> change it
Ah, right. That can certainly be improved. I added that check just before the
call that might get EAGAIN returned, but as you say - if total_read is
non-zero at that point it was non-zero already before the loop and the check
would be better placed already there.
Will you do the additional edits?
-- / daniel.haxx.se _______________________________________________ libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-develReceived on 2012-02-07