[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl: handle null lists in libxl_string_list_length
On Sat, Sep 28, 2013 at 12:28 AM, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote: > On Sat, 2013-09-28 at 00:20 +1200, Matthew Daley wrote: >> On Sat, Sep 28, 2013 at 12:08 AM, Boris Ostrovsky >> <boris.ostrovsky@xxxxxxxxxx> wrote: >> > >> > ----- mattjd@xxxxxxxxx wrote: >> > >> >> After commit b0be2b12 ("libxl: fix libxl_string_list_length and its >> >> only >> >> caller") libxl_string_list_length no longer handles null (empty) >> >> lists. Fix >> >> so they are handled, returning length 0. >> >> >> >> While at it, remove the unneccessary undereferenced null pointer >> >> check >> > >> > Are you sure this check should be removed? This routine can be called >> > from anywhere (at least within libxl it seems) and one day someone will >> > call it with NULL argument. >> > >> > I'd probably leave this check in. >> >> I would argue that any such invocation would be an error by the caller >> and should fail noisily, similar to how passing NULL into strlen >> should not return 0. libxl_{string,key_value}_list_dispose similarly >> assumes non-NULL pointers, FWIW. >> >> Ian C., do you have an opinion either way? > > I think a zero length list is a bit different to a NULL string and > should return 0. Perhaps it was a bad analogy, but passing NULL to this function isn't giving it an empty list, it's giving it no (NULL!) list. We don't check for null pointers everywhere else non-optional pointers are passed (at least, we shouldn't be, IMO...) > > But libxl_string_list is already char** so this function is taking > char***. The check for char *** == NULL, which is being removed, appears > to be unnecessary. A zero length list would be char ** == NULL, which > should be handled (and is I think?). char * == NULL would be a "" entry > in the string list. This was my intention in this patch; only the char *** == NULL check is removed, and the char ** == NULL for empty lists is handled by the newly added if condition. But char * == NULL doesn't mean an "" entry, doesn't it instead mark the end of the list (see xlu_cfg_get_list_as_string_list for example)? This is currently being checked for in the while loop condition. To continue using your notation, instead char == NULL is an empty string value in the list. > > Confused? I know I am ;-) :) - Matthew > > Ian. > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |