well, there are ~10 unhandled "malloc" and ~3 unhandled "calloc" in 51d.c
I'm against reviewing/fixing them altogether
can we fix it step by step instead ?
as for missing space, I can fix (or this can be fixed when applying)
пт, 3 янв. 2025 г. в 05:14, Willy Tarreau <w...@1wt.eu>:
> 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
>
