Hal Murray via devel <devel@ntpsec.org>:
> I know what's going wrong, but I'm not enough of a python geek to see a clean
> fix.
>
> The basic idea is that the client sends a request and the server sends back a
> clump of packets. The client specifies the max clump size. What's happening
> is that at least one packet of the clump is getting lost. The code is
> supposed to reduce the clump size when that happens, but that's not working.
>
> Here is the code outline:
> mrulist calls doquery to get a clump of data
> there is an except phrase to reduce the clump size
> doquery calls sendrequest then getresponse
> getresponse returns the answer in self.response
> it also returns None (and maybe falls off the end with no return?)
>
> I think getresponse needs to return two things.
> one is the data
> the second is a flag: none, some, all
>
> There are lots of raises and excepts in there that I haven't sorted out.
>
> I think the code should return partial data. It doesn't. Or it doesn't get
> processed.
>
> What's the right way to structure this? Should we fix the current code, or
> make a drastic change?
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.
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.
> ----------
>
> Where is the if for this else? Can else go with something other than if?
>
> The code past the else is the normal case. It gets run if the break doesn't
> happen.
>
> This chunk of code is from near the end of getresponse in pylib/packet.py
>
> # If we've seen the last fragment, look for holes in the sequence.
> # If there aren't any, we're done.
> if seenlastfrag and fragments[0].offset == 0:
> for f in range(1, len(fragments)):
> if fragments[f-1].end() != fragments[f].offset:
> warndbg("Hole in fragment sequence, %d of %d"
> % (f, len(fragments)), 1)
> break
> else: <=== this one
> tempfraglist = [ntp.poly.polystr(f.extension) \
> for f in fragments]
> self.response = ntp.poly.polybytes("".join(tempfraglist))
> warndbg("Fragment collection ends. %d bytes "
> " in %d fragments"
> % (len(self.response), len(fragments)), 1)
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.
--
<a href="http://www.catb.org/~esr/";>Eric S. Raymond</a>
_______________________________________________
devel mailing list
devel@ntpsec.org
http://lists.ntpsec.org/mailman/listinfo/devel
