berdario / hcobs

Haskell implementation of Consistent Overhead Byte Stuffing
BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

export `Stuffed` constructor or create `wrap` function #1

Open user16332 opened 11 months ago

user16332 commented 11 months ago

I think the Stuffed constructor of the newtype needs to be exported in the module header or we need a wrap function that's the opposite of unwrap.

The library seems to be geared towards sending COBS. However I am receiving COBS. I order to unstuff a raw ByteString received from the network I need to wrap it using the Stuffed constructor or there could be a wrap function if the constructor is considered internal implementation detail. Only then I can pass the Stuffed type to the unstuff function.

berdario commented 11 months ago

Indeed I didn't think of the receiving use case.

I'd prefer to avoid people accidentally using the constructor incorrectly (if you have a Stuffed 0, you know it was constructed correctly, and doesn't contain any 0 bytes)

alternatives:

I think the latter one is simpler, what do you think?

(btw, accepting a pull request might be easier/faster than waiting for me to implement it, I have a bunch of other things that I need to do 😓 )

user16332 commented 11 months ago

what's the danger of unstuffing a ByteString that has the forbidden bytes in it?

I just tried a quick'n'dirty putStrLn . show . unstuff . (Stuffed :: BL.ByteString -> Stuffed 0) . BL.pack $ [0x00] and it decoded just fine to an empty string without the computer going up in smoke.

also in reality my bytestrings obv. don't have forbidden bytes in them because they come fresh off the parsing routine that splits the raw bytestream into packets along the marker byte separator.

but I'll leave it to you if you want to introduce that extra check.

if you go the Internal module path, the module should probably rather be called Unsafe in line with Haskell naming tradition. Internal is somehow for exposing implementation details that can change at any time which this isn't, or is it?

suggestion 2 or 3 would both have the same check would they? I guess wrap would be more consistent as it's the opposite of the existing unwrap but this is bordering on bikeshedding.