Hi Ilya,
On Thu, Jan 02, 2025 at 10:02:01PM +0100, ???? ??????? wrote:
> ??, 2 ???. 2025 ?. ? 21:46, Miroslav Zagorac <z...@fly-etf.net>:
>
> > On 02. 01. 2025. 21:40, ???? ??????? wrote:
> > > Honestly, I think those elements must be deallocated on program exit,
> > > not only if something failed during allocation.
> > >
> > > but I did not check that
> > >
> >
> > That is correct. However, the calloc() result is not checked before
> > strdup()
> > either, so the patch is not good.
> >
>
> I did not pretend to add "calloc" check for this patch.
> we have dedicated *dev/coccinelle/unchecked-calloc.cocci *script which
> allows us to detect unchecked "calloc". no worry, it won't be forgotten
In general, when reworking some functions' memory allocation and checks,
it's better to fix it at once when you find that multiple checks are
missing, rather than attempting incremental fixes that remain partially
incorrect.
One reason is that very often, dealing with allocations unrolling requires
an exit label where allocations are unrolled in reverse order, and doing
them one at a time tends to stay away from that approach. Or sometimes
you'll figure that fixing certain unchecked allocations require to
completely change the approach that was used for previous fixes.
Thus if you think you've figured how to completely fix that function, do
not hesitate, please just fix it all at once, indicating in the commit
message what you fixed. If you think you can fix it incrementatlly without
having to change your fix later, then it's fine to do it that way as well
of course.
> > >>> + if (name->name == NULL) {
> > >>> + memprintf(err,"Out of memory.");
^^^^
BTW, beware of the missing space here.
> > >>> + goto fail_free_name;
> > >>> + }
Cheers,
Willy
