4 ms·
The C code in the article is not great, but I don't think the author was trying to write a robust, production-ready Wayland client either. A couple examples of
by sph 3y ago
The C code in the article is not great, but I don't think the author was trying to write a robust, production-ready Wayland client either.
A couple examples of fragile code, so my comment is a little constructive:
- I truly dislike the cstring_len macro, which sounds like strlen yet its semantics are completely different.
- The wayland_display_connect function returns EINVAL or an fd. Both are positive integers. It should instead return -EINVAL.
- The functions benefit from more comments, because they do a lot and are not factored to smaller, more composable units.
- wayland_handle_message should be refactored so it's a simple switch that operates on constants, and ideally delegates to other functions.
- Probably outside of the scope of a simple article, but all the buffer boilerplate should be simplified by using a buffer structure that holds pointer, cursor and capacity, to make the code much more readable.
- ahoka 3y agoAnd EINVAL is a perfectly valid fd.
- mananaysiempre 3y ago> wayland_handle_message should be refactored so it's a simple switch that operates on constants, and ideally delegates to other functions. Is that actually possible? Opcodes by themselves are not unique, and the object IDs are right now assigned at runtime (I can’t tell if that’s actually necessary).
- unwind 3y agoAgree. Not sure about roundup_4() either.
- smcameron 3y ago> The wayland_display_connect function returns EINVAL or an fd. Both are positive integers. It should instead return -EINVAL. Or maybe return -1 and set errno? I haven't seen many C programs that return negative errnos (excepting the linux kernel which uses negative errnos internally).
- dundarious 3y agoWhy continue with the mistakes of the C standard and go through a thread-local global variable instead of just a return value? Convention is often not a good justification.
- charcircuit 3y agoNot being production ready isn't an excuse when you are writing code that is intended to be copied by others.
- sph 3y agoI disagree. Didactic code doesn't have to be complete, just clear enough to teach. Nor the teacher is not responsible for lazy students. It's lazy and quite ignorant to criticize teaching material because it doesn't cover all possible corner cases, and because it doesn't account for a reader that just wants to copy the code without understanding it.
- charcircuit 3y agoThere is a difference between not handling a corner case and having a security vulnerability. People are lazy and they will copy and paste without thinking.
- sph 3y agoWhose fault would that be? Geez, now blog authors need to have disclaimers, like reminding people that, no, you shouldn't eat Tide pods. The topic is Wayland, not C code hardening techniques for internet facing services. Quit being an obnoxious pedant.