-
-
Notifications
You must be signed in to change notification settings - Fork 8
Store contents of static string struct in the array buffer #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't have a thorough look through, but I like this, thanks.
// Allocates a new string buffer for *out* with enough capacity to store | ||
// *num_bytes* of text. The actual allocation is num_bytes + 1 bytes, to | ||
// account for the null terminator. Does not do any initialization, the caller | ||
// must initialize and null-terminate the string buffer. Errors if *out* | ||
// already contains data or if malloc fails. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 This is super valuable documentation. Thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reducing the number of stars everywhere makes this so much more readable, thanks for this! ⭐
This is a followup for #41, since I had to make a number of extensive changes relative to what that PR originally proposed, I issued a new PR instead to hopefully make this clearer.
Instead of using flexible-length arrays, the
ss
struct now looks like this:Rather than store a pointer to an
ss
struct in the array buffer, this refactors things so we instead store the contents of thess
struct in the array buffer. This reduces the amount of pointer indirection we need to do to get access to thess
struct data and also makes it much easier to deal with empty strings, since with this new definition((ss*)NULL)->len
is zero by construction.Instead of
malloc
ing storage for the entire struct on the heap, we now onlymalloc
space for the string buffer, and the caller of functions that allocate memory are responsible for handling memory for thess
struct.To handle this change, I had to rewrite the static_string API to accept pointers to
ss
instances instead of returning them. I also added a newload_string
helper that returns either a read-only view onto anss
struct, or if the struct isNULL
, returns a view onto a statically allocated empty struct.