On Tue, Aug 31, 2021 at 02:47:01PM +0200, Ævar Arnfjörð Bjarmason wrote: > > That works, but now "util" is not available for all the _other_ uses for > > which it was intended. And if we're not using it for those other uses, > > then why does it need to exist at all? If we are only using it to hold > > the allocated string pointer, then shouldn't it be "char *to_free"? > > Because having it be "char *" doesn't cover the common case of > e.g. getting an already allocated "struct something *" which contains > your string, setting the "string" in "struct string_list_item" to some > string in that struct, and the "util" to the struct itself, as we now > own it and want to free() it later in its entirety. OK. I buy that storing a void pointer makes it more flexible. I'm not altogether convinced this pattern is especially common, but it's not any harder to work with than a "need_to_free" flag, so there's no reason not to do that (and to be fair, I didn't look around for possible uses of the pattern; it's just not one I think of as common off the top of my head). > That and the even more common case I mentioned upthread of wanting to > ferry around the truncated version of some char *, but still wanting to > account for the original for an eventual free(). > > But yes, if you want to account for freeing that data *and* have util > set to something else you'll need to have e.g. your own wrapper struct > and your own string_list_clear_func() callback. But stuffing it into the util field of string_list really feels like a stretch, and something that would make existing string_list use painful. There are tons of cases where util points to some totally unrelated (in terms of memory ownership) item. I'd venture to say most cases where string_list_clear() is called without free_util would count here. > > I don't think most interfaces take a string_list_item now, so wouldn't > > they similarly need to be changed? Though the point is that all of these > > degrade to a regular C-string, so when you are just passing the value > > (and not ownership), you would just dereference at that point. > > Sure, just like things would need to be changed to handle your proposed > "struct def_string". > > By piggy-backing on an already used struct in our codebase we can get a > lot of that memory management pretty much for free without much > churn. > > If you squint and pretend that "struct string_list_item" isn't called > something to do with that particular collections API (but it would make > use of it) then we've already set up most of the scaffolding and > management for this. It's that squinting that bothers me. Sure, it's _kinda_ similar. And I don't have any problem with some kind of struct that says "this is a string, and when you are done with it, this is how you free it". And I don't have any problem with building the "dup" version of string_list with that struct as a primitive. But it seems to me to be orthogonal from the "util" pointer of a string_list, which is about creating a mapping from the string to some other thing (which may or may not contain the string, and may or may not be owned). TBH, I have always found the "util" field of string_list a bit ugly (and really most of string_list). I think most cases would be better off with a different data structure (a set or a hash table), but we didn't have convenient versions of those for a long time. I don't mind seeing conversions of string_list to other data structures. But that seems to be working against using string_list's string struct in more places. -Peff
