Re: [PATCH 17/22] reftable/iter: handle allocation failures when creating indexed table iter

> Patrick Steinhardt <ps@pks.im> writes:
> 
> > -int new_indexed_table_ref_iter(struct indexed_table_ref_iter **dest,
> > +int indexed_table_ref_iter_new(struct indexed_table_ref_iter **dest,
> >  			       struct reftable_reader *r, uint8_t *oid,
> >  			       int oid_len, uint64_t *offsets, int offset_len)
> >  {
> >  	struct indexed_table_ref_iter empty = INDEXED_TABLE_REF_ITER_INIT;
> > -	struct indexed_table_ref_iter *itr = reftable_calloc(1, sizeof(*itr));
> > +	struct indexed_table_ref_iter *itr;
> >  	int err = 0;
> >  
> > +	itr = reftable_calloc(1, sizeof(*itr));
> > +	if (!itr) {
> > +		err = REFTABLE_OUT_OF_MEMORY_ERROR;
> > +		goto out;
> > +	}
> > +
> >  	*itr = empty;
> >  	itr->r = r;
> >  	strbuf_add(&itr->oid, oid, oid_len);
> > @@ -197,11 +203,15 @@ int new_indexed_table_ref_iter(struct indexed_table_ref_iter **dest,
> >  	itr->offset_len = offset_len;
> >  
> >  	err = indexed_table_ref_iter_next_block(itr);
> > -	if (err < 0) {
> > +	if (err < 0)
> > +		goto out;
> > +
> > +	*dest = itr;
> > +	err = 0;
> > +
> > +out:
> > +	if (err < 0)
> >  		reftable_free(itr);
> > -	} else {
> > -		*dest = itr;
> > -	}
> >  	return err;
> >  }
> 
> Unless the service the helper function offers is to upgrade an
> existing resource (e.g., realloc() taking a pointer and give an
> enlarged piece of memory), it may be a safer calling convention to
> promise that *dest is cleared to NULL when the function fails,
> instead of promising that *dest is left intact.  The caller, when it
> needs to evantually release the resource acquired here, has to
> remember what the returned value (i.e., err) was, in order to decide
> if it needs to call the release helper on *dest it obtained from us.
> 
> The only caller seems to initialize *dest to NULL itself, so it does
> not matter in the current code, though.