ntpq mrulist bug
hmurray at megapathdsl.net
Wed Nov 20 23:44:27 UTC 2019
> 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
> 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
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.
More information about the devel