On Wed, 12 Oct 2022 12:01:54 -0400 Gregory Price <gregory.pr...@memverge.com> wrote:
> This code contains heap corruption on free, and I think should be > refactored to pre-allocate all the entries we're interested in putting > into the table. Good point on the heap corruption.. (oops. Particularly as I raised that I didn't like the complexity of your free in your previous version and still failed to notice the current code was wrong...) > This would flatten the code and simplify the error > handling steps. I'm not so keen on this. Error handling is pretty trivial because of the autofree magic. It will get a tiny bit harder once we have two calls to the factored out function, but not too bad - we just need to free the handed off pointers in reverse from wherever we got to before the error. > > Also, should we consider making a union with all the possible entries to > make entry allocation easier? It may eat a few extra bytes of memory, > but it would simplify the allocation/cleanup code here further. An interesting point, though gets trickier once we have variable numbers of elements. I'm not sure it's worth the effort to save a few lines of code. > > Given that every allocation has to be checked, i'm also not convinced > the use of g_autofree is worth the potential footguns associated with > it. After rolling a version with some of your suggested changes incorporated the autofree logic is all nice and localized so I think it's well worth having. Only slightly messy bit is we end up with 4 separate pointers for the bandwidth and latency elements.
