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