4 ms·
Then uv_loop_t should be an opaque type so its size isn't known at all, and uv_loop_init() should instead be uv_loop_new() returning the heap-allocated uv_loop_
by newnewpdro 8y ago
Then uv_loop_t should be an opaque type so its size isn't known at all, and uv_loop_init() should instead be uv_loop_new() returning the heap-allocated uv_loop_t, and uv_loop_close() renamed to uv_loop_free().
As-is the API and the way its use is being demonstrated in that example appear amateur to say the least.
- viraptor 8y agoBut then you wouldn't be able to stack allocate it. It's reasonable to make something recommended, but not forbid other approaches if someone really wants to do it that way. You don't have to go to either extreme.
- TomMarius 8y agoI wouldn't call a library that is the foundation of one of the most popular platforms in the world amateur.
- newnewpdro 8y agoEver looked at the PHP implementation throughout its popular existence? Popularity is an awful metric for gauging implementation quality.
- TomMarius 8y agoAgreed, however PHP was never praised for anything other than its ecosystem. Node is generally praised for its async IO, which is built on top of libuv.
- megous 8y agoFunny how most structs in the linux kernel are public, they must be all amateurs, too. Perhaps go read about container_of and struct embedding in C before calling anyone an amateur. It's just a different programming approach in C. Encapsulation is good only for making it harder for anyone to poke in the internals of your library, which has some benefits with binary compatibility, etc. But it's strictly worse in all other metrics when coding in C. It limits you, it forces malloc calls where most of the time none would be necessary, it makes memory management more complicated, it forces explict initialization, etc.
- dap 8y agoIf one already accepts that the object should be dynamically allocated because otherwise it's too easy to accidentally copy (which seems specious, but that was the suggestion), then all of those arguments apply to that recommendation, too. And if it's "vital" that the object not be copied, then the API could enforce that with an opaque type. I've only worked with libuv in a few contexts, and maybe there are compelling reasons for exposing the struct. (I'd like to hear them! I would not have expected a uv_loop to be allocated in perf-critical code paths or code paths where failure isn't an option.) But I think critical analysis of C API design is an important topic. C gets a bad rap for being unsafe, which it obviously is in many ways, but as C developers, simple choices like this (that a novice might not even think much about) can make an API much safer -- or much less safe.
- megous 8y agoPerformance is orthogonal to this. To me the less you juggle with pointers and malloc/free the safer your code will be from memory leaks, misuse of unallocated memory, NULL checking issues, and easier to inspect/reason about. Struct embedding helps with this quite a bit. And it's not possible without exposing the struct definition. Performance gains are possible, but that's secondary. > If one already accepts that the object should be dynamically allocated It can still be allocated on heap, but in one continuous chunk of memory as a part of the larger struct. Hiding the struct definition would prevent this.
- dap 8y ago> Performance is orthogonal to this. That's largely true, but it's often cited as a reason to avoiding malloc/free (however dubious that is). > To me the less you juggle with pointers and malloc/free the safer your code will be from memory leaks, misuse of unallocated memory, NULL checking issues, and easier to inspect/reason about. I don't quite agree. I've worked mostly in code bases using the pattern described earlier (an opaque pointer, a $type_create(), and a $type_destroy() function). With that pattern, I find it much easier to be certain by code inspection that a particular object or transformation is valid because as long as the pointer was allocated correctly, the object can only be modified by functions that know the type (aside from memory corruption, but that's always possible). That's usually a small set of functions that know the struct details. This fact is useful both as a library author and as the author of a library consumer. By contrast, if the struct is exposed, it's harder to identify all the places that can modify the structure's details and to be sure that invariants are maintained in all those places. Besides that, several other failure modes are much less likely with opaque structures, including copying a structure you shouldn't, miscopying a structure that's okay to copy (e.g., off-by-one while copying), or operating on a correctly-sized block of memory that's never been initialized. You can still use unallocated memory, of course, but that's fairly easy to make safer by initializing pointers to NULL. (The analogous option for stack-initialized structs -- initializing them to zero -- is not necessarily any safer than leaving them uninitialized -- particularly if the struct contains file descriptors.) There are tradeoffs to both approaches. To me, the ability to modify the struct in new versions of the library without breaking the ABI is a pretty major point in favor of using an opaque structure for a library's primary handle. For very simple ancillary structures that are very unlikely to change, and where the convenience of stack allocation is worthwhile, exposing the structure makes a lot of sense.