Re: [Libguestfs] [PATCH 08/11] ocaml: do not try to malloc 0 elements in get_all_event_callbacks

On Fri, Mar 03, 2017 at 03:33:02PM +0100, Pino Toscano wrote: > In case there are no event handlers registered with the handle, > get_all_event_callbacks will count 0 elements, trying to malloc a buffer > of that size. POSIX says that this can result in either a null pointer, > or an unusable pointer. Since we assume a null pointer means failure, > then always add a null element at the end, so we do not rely on > implementation-defined behaviour of malloc. > > The output parameter 'len_rtn' already keeps the number of valid items > in the returned array, so there are no behaviour changes for callers of > get_all_event_callbacks. > --- > ocaml/guestfs-c.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/ocaml/guestfs-c.c b/ocaml/guestfs-c.c > index 9042752..f14eee3 100644 > --- a/ocaml/guestfs-c.c > +++ b/ocaml/guestfs-c.c > @@ -311,7 +311,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) > } > > /* Copy them into the return array. */ > - r = malloc (sizeof (value *) * (*len_rtn)); > + r = malloc (sizeof (value *) * (*len_rtn + 1)); > if (r == NULL) caml_raise_out_of_memory (); Isn't it better to fix this by doing: r = malloc (sizeof (value *) * (*len_rtn)); - if (r == NULL) caml_raise_out_of_memory (); + if (*len_rtn > 0 && r == NULL) caml_raise_out_of_memory (); (same comment in the following patches)