On Mon, Sep 30, 2024 at 07:40:48PM +0200, René Scharfe wrote:
> Am 30.09.24 um 10:08 schrieb Patrick Steinhardt:
> > diff --git a/reftable/writer.c b/reftable/writer.c
> > index ed61aaf59c..54ec822e1c 100644
> > --- a/reftable/writer.c
> > +++ b/reftable/writer.c
> > @@ -117,13 +117,17 @@ static void writer_reinit_block_writer(struct reftable_writer *w, uint8_t typ)
> > w->block_writer->restart_interval = w->opts.restart_interval;
> > }
> >
> > -struct reftable_writer *
> > -reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
> > - int (*flush_func)(void *),
> > - void *writer_arg, const struct reftable_write_options *_opts)
> > +int reftable_writer_new(struct reftable_writer **out,
> > + ssize_t (*writer_func)(void *, const void *, size_t),
> > + int (*flush_func)(void *),
> > + void *writer_arg, const struct reftable_write_options *_opts)
> > {
> > - struct reftable_writer *wp = reftable_calloc(1, sizeof(*wp));
> > struct reftable_write_options opts = {0};
> > + struct reftable_writer *wp;
> > +
> > + wp = reftable_calloc(1, sizeof(*wp));
> > + if (!wp)
> > + return REFTABLE_OUT_OF_MEMORY_ERROR;
> >
> > if (_opts)
> > opts = *_opts;
> > @@ -134,13 +138,19 @@ reftable_new_writer(ssize_t (*writer_func)(void *, const void *, size_t),
> > strbuf_init(&wp->block_writer_data.last_key, 0);
> > strbuf_init(&wp->last_key, 0);
> > REFTABLE_CALLOC_ARRAY(wp->block, opts.block_size);
> > + if (!wp->block) {
> > + free(wp);
>
> Better use reftable_free() to free it, since you use reftable_calloc()
> to allocate it above.
>
> Perhaps ban free(3), strdup(3) etc. at the end of reftable/basics.h,
> banned.h style?
Ugh. I was relying too much on your review having mentioned all cases,
but I really should've double-checked the other patches, too. Mind you,
I really don't mean to blame you here, I blame myself.
In any case, banning these functions via "reftable/basics.h" certainly
seems like a good idea. It's just too easy to screw up by accident. Will
fix tomorrow.
Thanks!
Patrick
