glnvg__alloc not respecting requested allocation size

[memononen]

glnvg__alloc not respecting requested allocation size

[memononen]

Excellent catch! Sorry for the trouble finding it. I changed all allocators to follow same pattern in the hopes that copy-pastering will not catch a bad habit in the future. Just to be sure, can you please review the changes and let me know if there is something silly with it.

[cforfang]

Just thinking out loud for a bit, I was wondering if the use of realloc might cause some issues?

The glnvg__alloc*s all use it like this:

gl->paths = (struct GLNVGpath*)realloc(gl->paths, sizeof(struct GLNVGpath) * gl->cpaths);

... which if realloc fails all the old data is leaked and you're probably looking at a segfault in the near future?

Similar issues in nvg_add* and nvg__appendCommands where you do check for NULL but still leak the old data -- and by not clearing e.g. ncommands/ccommands I guess the code is likely to try to dereference the NULL later though I haven't looked at it closely.

I guess realloc is pretty unlikely to fail on modern system, at least for what its used for here, but still might be something to consider. :)

[memononen]

You're right. So pattern like would be better:

struct GLNVGpath* paths;
paths = (struct GLNVGpath*)realloc(gl->paths, sizeof(struct GLNVGpath) * gl->cpaths);
if (paths == NULL) fail;
gl->paths = paths;

I think the code should try to fail as close to the problem as possible, even if it is a segfault. So that code definitely needs a little improvements.

[cforfang]

Yeah. What you could maybe do is (at least for the gl-backend) change glnvg__alloc* to somehow indicate allocation-failure, and then handle this in glnvg__render* by allocating everything up front (with glnvg__allocCall last) and either silently dropping the draw or reporting it through an error-callback or something. Not sure how complex you want to make this though: :)

[memononen]

Take a look at 7f55dec I hope caught all cases.

[cforfang]

The only thing that jumps out at me is that you probably shouldn't update the capacity before you know the realloc succeeds. Will run into problems the second time the function is called. :) Looks good otherwise!

[memononen]

One more time... 02ca24a
Remote pair programming ftw! :)

[cforfang]

Indeed. :) Seems, compiles, and runs fine for me.

[guidodehaan]

Realloc changes look ok. And running without problems now.