pypcap icon indicating copy to clipboard operation
pypcap copied to clipboard

Improve the documentation for __next__()

Open JosiahDub opened this issue 6 years ago • 5 comments

After 924dfdeec4d2a045d79f06a80edb96948f0f1184, __next__() continues instead of returning None when timing out. This does not seem like ideal behavior. Am I misinterpreting timeout or misusing the function? Ideal code snippet:

import pcap
p = pcap.pcap(timeout_ms=50)
# Set the filter so that no packets will arrive
p.setfilter("port 55555")
try:
    timestamp, packet = p.__next__()
except TypeError:
    print("__next__() returned None in a reasonable time-frame!")

I modified pcap.pyx to return None and the code snippet worked.

JosiahDub avatar May 22 '18 14:05 JosiahDub

@nuald what are you thoughts on this?

hellais avatar Sep 04 '18 09:09 hellais

I've already mentioned it here: https://github.com/pynetwork/pypcap/pull/47/files#r116732203 The loop should be infinite - it's the user responsibility to correctly exit the loop. The timeout_ms parameter corresponds to packet buffer timeout (see https://www.tcpdump.org/manpages/pcap.3pcap.html), not to the timeout for exiting the loop if nothing happens. I'd recommend to update the documentation or provide additional timeout parameter to avoid confusion.

nuald avatar Sep 05 '18 00:09 nuald

Ah right, thanks for reminding me of that. I would say then this ticket should become that of updating the docs to document this as the expected behaviour.

hellais avatar Sep 17 '18 16:09 hellais

@nuald But why not give users a choice to handle the event, like raise TimeoutError or something else?

wtdcode avatar Apr 16 '20 06:04 wtdcode

@wtdcode Sorry, I'm not working on this project nowadays, and only can give some advice: please feel free to modify pcap.pyx for your needs - the packet buffer timeout occurs when pcap_ex_next returns 0 (please see https://www.tcpdump.org/manpages/pcap_next_ex.3pcap.html for the details). Right now in the code there is no special processing for it, and I guess you could add something there instead of just calling continue.

nuald avatar Apr 16 '20 18:04 nuald