bos / pcap

Haskell bindings for the pcap library, which provides a low level interface to packet capture systems.
http://bitbucket.org/bos/pcap
Other
27 stars 11 forks source link

Make `dump*` functions usable, add `breakLoop`, improve docs, and expand examples #10

Open ntc2 opened 7 years ago

ntc2 commented 7 years ago

I can break this up into separate pull requests if prefer. I've used many small commits to make it easy to see what I changed, and the changes are also well summarized in the CHANGELOG.md I added.

My original motivation was to make the dump* functions usable via the provided APIs, i.e. Issue #3.

The documentation and type changes for the loop* functions fix Issue #4.

@bos: I see that there haven't been any commits in this repo since 2012, and there are several outstanding pull requests. Are you still maintaining this project?

ntc2 commented 7 years ago

@bos Bump! In case you are not still maintaining this project, I would be happy to take over for you. It seems pretty low volume, and we're using this library in a project at Galois for the next year or so.

Gabriella439 commented 6 years ago

@bos: Please merge this or give somebody else the commit bit. I just ran into several issues that this pull request fixed

m00ngoose commented 5 years ago

@ntc2 given that I'm building against your PR, perhaps it makes more sense to ask you rather than @bos , what do you think of providing an unsafe variant of the BS functions? It's a simple enough modification, but converting eg. the Ptr Word8 returned by next into an (uncopied) ByteString seems like a common use-case.

ntc2 commented 5 years ago

what do you think of providing an unsafe variant of the BS functions? It's a simple enough modification, but converting eg. the Ptr Word8 returned by next into an (uncopied) ByteString seems like a common use-case.

@awhtayler Sorry, I don't understand. Can you provide more detail or some example code?

m00ngoose commented 5 years ago

loopBS and dispatchBS call wrapBS, nextBS calls toBS.

wrapBS and toBS both have a line that reads essentially bs <- B.create (fromIntegral len) $ \p -> B.memcpy p ptr (fromIntegral len) - the packet data is copied from its location ptr into a newly created ByteString. This is done because the reference is potentially invalidated between calls (see here - "The struct pcap_pkthdr and the packet data are not to be freed by the caller, and are not guaranteed to be valid after the next call to pcap_next_ex(), pcap_next(), pcap_loop(3PCAP), or pcap_dispatch(3PCAP); if the code needs them to remain valid, it must make a copy of them.")

However, in the vein of eg. Conduit's sourceHandleUnsafe, if you're going to process the ByteString in such a way that you need not maintain references to it between calls to nextBS / running the callback provided to loopBS/dispatchBS, then there's no need to make a copy, and the line may as well instead read bs <- BU.unsafePackCStringLen (castPtr ptr, fromIntegral len) - treat the memory location pointed to by ptr as a ByteString directly.

ntc2 commented 5 years ago

@awhtayler thanks for explaining. I don't think it makes sense to add this unsafe functions as part of this PR. But you could create another PR that builds on top of this one, that adds those unsafe functions.