ntpq mrulist bug

[Hal Murray]

> I don't think I know yet.  I lean towards an incremental fix along the  lines
> you describe, but it's also possible that there's a serious design flaw that
> merits a rewrite.

The 20,000 ft view looks reasonable, but I think the use of signals is 
unreasonably complicating the control flow.  A signal would be reasonable for 
the no-response case, but that's unlikely since we already made contact with 
the server to get the cookie.  The hard case is some data but not all of it.  
You want to return both the data and an indication that a packet was lost.

Part of the problem is that there is a lot of cruft in that area.  For 
example, grep for CERR_
There is a clump of signals defined as part of a ControlSession, none are ever 
raised, a few are caught.  Looks like somebody decided to rename things to 
SERR and never got around to finishing the cleanup.

There is another case were stuff is returned a couple of layers, but then 
never used used.


> Please file an issue and assign it to Ian Breune; he's the maintainer for the
> Python parts.  I'll step in if he needs help. 

I filed one ages ago.  It's #547.  Looks like Ian is already the maintainer.

Ian: Poke me off list if you need help.  I have an annoyingly reproducible 
test case.

------

[for, else]
> That's a Pythonism.  An else clause attached to a for executes only if the
> for ran to complewtion without a break.  In this case, the code checks for a
> hole in the fragment sequence and sets the response field if there is no
> hole. 

Thanks.

You have a tendency to use legal but uncommon constructs.  Is that a bug or 
feature?  On the feature side, it makes the code more compact and maybe some 
of us learn something.  On the bug side, it makes the code harder to 
understand for those of us who don't mentally collect features.

Is there a collection of obscure features and what they do?  I'd like to scan 
(and bookmark) something like that.

In that particular example, I think the break could have been replaced with a 
return and the else and indent of the following clump removed.  It may be 
slightly more complicated than that and/or I may be misreading things.  There 
is a loop to get another packet.  That chunk of code is processing the 
last-packet case, but it's still inside the loop.  I think it would be cleaner 
to exit the loop first, then do that batch of processing.

-- 
These are my opinions.  I hate spam.