Re: [Xen-devel] [PATCH] libxl: handle null lists in libxl_string_list_length

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-09-27 at 09:28 -0400, Boris Ostrovsky wrote:
> 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
> >>>>> 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...)
> 
> 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?

This is the distinction Matthew are I were trying to make. In the case
you describe you would do libxl_string_list psl = NULL (a zero length
argument list) and call libxl_string_list_length(&psl).

libxl_string_list_length(NULL) is not asking for the length of a
zero-length, list, it's asking for the length of no-list at all. If this
were floating point the answer would be NaN ;-) Instead we get a
segfault...

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel