On Tue, May 23, 2023 at 11:50:04AM +0200, Claudio Jeker wrote:
> Calling malloc() with a zero length is entering underspecified territory.
> So ibuf_open(0) but more importantly ibuf_dynamic(0, max) step right into
> that trap. In the first case the call makes little sense and we should
> error out. In the second case it is better to skip the allocation of the
> buffer and have a later ibuf_realloc() allocate space.

Apart from the regress test that you just fixed I can't find a
deliberate call to imsg_open(0). For most callers it's very clear that
len > 0 and all callers check, so this change seems fine.

> Also ensure errno is always set when erroring out in ibuf_open() and
> ibuf_dynamic().
> 
> While changing this code use calloc() for the buffers so they are 0
> initialized by default. ibuf_realloc() already uses recallocarray()
> so the code was halfway there.
> 
> And since we are pushing the initialize memory train also do a memset() in
> ibuf_reserve(). Now this part is probably more like belt and suspenders
> since the buffer is already 0 (unless the caller somehow scribbled into it).
> At least iked includes code to do this memset() but I'm happy to remove
> this bit.

I think it doesn't hurt to do this last memset().

ok tb

Reply via email to