search

huynguyen-and-friend-projects / smoldb

Simple database in C(++)
1 stars 3 forks source link

Change InputBuf from an opaque pointer to a fully-defined struct #32

Closed nguyenhuy0905 closed 3 months ago

nguyenhuy0905 commented 3 months ago

I changed InputBuf from an opaque pointer to a fully-defined struct.

The heap-allocated functions for InputBuf are still kept, for obvious reason.

NOTE: ONLY REVIEW THIS PR AFTER #28, #27, #26 and #25

One can now stack-allocate an InputBuf. This can make the code easier to read, and saves a couple assembly instructions I suppose.

However, this comes at the cost of potential skill-issue when functions not belonging to InputBuf's API(?) directly modify an InputBuf's state.

They are free to get data directly from InputBuf, but should not modify them without API calls.

I simply moved the definition of InputBuf from the .c file to the .h file.

This is a kind-of breaking change. While one can still use the good old heap-allocated InputBuf, it should only be done so if the InputBuf's lifetime needs to be longer than the function that defined it or struct that contains it (eg, passing InputBuf between functions/structs).

If a function takes an InputBuf, and you have a stack-based InputBuf, pass a reference, don't pass an InputBuf by value. Passing InputBufs by value can cause confusion, since the InputBuf's inner character array is still heap-allocated.

@BAOELIETRAN @qu-ngx @AbstractionHLR @bebeminhminh

Pull request by: Huy Nguyen

nguyenhuy0905 commented 3 months ago

Well, you can say this PR is quite opinionated also. Does one prefer safety, or the niceties of stack allocation. I would argue that this one is fine to let the users know what's inside, since the alternative is somewhat similar anyways (getter/setter). If I have a struct that I just want the user to hold a reference to (like the FILE struct; you don't need to know anything inside. It just works), I could use the old way (typedef in header, define in source).

nguyenhuy0905 commented 3 months ago

Scrap it. I don't think this is a very nice idea. Maybe I will add (yet) another header file (that isn't exposed) and put the struct definition inside. That way, what is exposed to the API is just an opaque pointer, while the selected source files can still stack-allocate InputBuf.