4 ms·
Generally, you only create one uv_loop_t, and it is vital it is never copied (which is easy to do accisentally if you stack allocate it). mallocing is the enti
by CJefferson 8y ago
Generally, you only create one uv_loop_t, and it is vital it is never copied (which is easy to do accisentally if you stack allocate it).
mallocing is the entirely sensible thing to do here.
- newnewpdro 8y agoThen 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.
- 8y ago
- int0x80 8y agoWhat do you mean by accidentally copy? struct embbed { ... uv_loop_t loop; ... } e1, e2; ... e1 = e2; ?
- newnewpdro 8y agoPresumably they're referring to copy via simple assignment vs. requiring a memcpy or pointer dereference. It's a b.s. argument.
- int0x80 8y agototally. Your comment was right.
- beached_whale 8y agoI just assumed it was because if the memory location ever changes it would invalidate all the pointers to it. It has to have a fixed memory location