[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 09/27/2013 09:14 AM, Matthew Daley wrote: 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 checkAre 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...) What if someone assigns 'libxl_string_list *psl = NULL' if, say, main()'s argc is 1 (i.e. there is no argument list) and then, later, calls libxl_string_list_length(psl) to find out whether something needs to be allocated for the list. Isn't getting a zero back an expected answer? (I am afraid we are approaching rathole territory here.) -boris 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 ;-):) - MatthewIan. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |