jinja template notes

[whot]

Not an issue, but I can't comment on files themselves so feel free to close this any time:

• strip_prefix: do you want this or something like this in the scanner? Maybe interface.basename or interface.plainname or something?
• {% if interface.name.startswith('eis_') %} I think it may be better to pass an arg to the scanner and use that instead of name parsing, feels more robust. the C bindings use a --component=ei or eis and then if on that.
• the match interface { in impl Request (last one in the template) has the _ => None condition inside the for loop, needs to be moved outside
• macro incoming_enum(interface) this snippet seems like something the scanner should provide, somehow anyway.
• {# TODO how to handle type that's not statically known? #}: you don't need to, if that ever happens error out and disconnect. you could skip until message.length and continue but why would you. This could only happen if the other side uses a version you didn't agree to and/or your client uses a interface version higher than the library supports (if the version is set-able by the client but then you can still min() it or return an error there).
• {# TODO validate that ID is valid? #} you can apply the mask for basic checking. and check that ids only ever go up.
• {# TODO fds; assume bytes read is total? #} not sure what you mean here but: any message parser function must read message.length bytes, but there may be more bytes sitting on the fd after that.

[ids1024]

Yeah, something like {{interface.basename}} could help.

you don't need to, if that ever happens error out and disconnect. you could skip until message.length and continue but why would you.

This else case is actually handling a new_id argument without an interface. Looks like the only instance of that is <arg name="object" type="new_id" interface_arg="interface_name" /> on the ei_device::interface event.

Otherwise yeah, things like this are protocol errors. I need to add an Error enum and add variants for various things.

not sure what you mean here but: any message parser function must read message.length bytes, but there may be more bytes sitting on the fd after that.

I need to check exactly how sendmsg and recvmsg on a Unix stream socket as far as partial reads, writes, and what happens with cmesgs in such cases...

And then there's also handling EINTR, which the sendmsg and recvmsg man pages say those functions can return. It looks like libei doesn't currently handle that, which at least in theory could be a problem.

[ids1024]

Looks like libei also doesn't check the return value of sendmsg to make sure that all bytes were sent, and re-send remaining bytes if not all were. Which can also happen in certain cases. (Low level Unix functions like this are a pain to deal with...)

I guess if EINTR is returned, the same send should be repeated, while if it returns a length less than what it attempted to send, it should send the remaining bytes and not the cmesgs.

[whot]

strip_prefix: do you want this or something like this in the scanner? Maybe interface.basename or interface.plainname or something?

ftr: libei's main branch now supports interface.plainname

Looks like libei also doesn't check the return value of sendmsg to make sure that all bytes were sent, and re-send remaining bytes if not all were. Which can also happen in certain cases. (Low level Unix functions like this are a pain to deal with...)

yeah, a lot of that low-level part was written quickly so i'm hoping these things can get fixed over time. Having said that, I wonder how many of these instances occur in our expected use-cases. If this some web-related server library I'd be worried about it but on your average desktop?

[ids1024]

EINTR only occurs if a signal occurs when the system call is being run. In most cases the ei client or server isn't receiving signals, and when it does is going to terminate anyway. (Depending on how signals are being used.)

I'm somewhat less sure when a partial write would occur with sendmsg in this context. The POSIX standard doesn't say so you'd have to check the implementation in the Linux kernel (and BSDs, etc.) to be sure.

So yeah, probably unlikely to actually be an issue, and not critical. But something that should be done more correctly.