Re: [Xen-devel] [PATCH v6 for-4.5 4/5] xl: add 'trim' and 'split_string_into_pair' functions

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Wei Liu writes ("Re: [PATCH v6 for-4.5 4/5] xl: add 'trim' and 
'split_string_into_pair' functions"):
> On Wed, Sep 24, 2014 at 09:48:04PM +0100, David Scott wrote:
> > +/* NB: this follows the interface used by <ctype.h>. See 'man 3 ctype'
> > +   and look for CTYPE in libxl_internal.h */
> > +typedef int (*char_predicate_t)(const int c);
> > +
> > +static void trim(char_predicate_t predicate, const char *input, char 
> > **output) __attribute__ ((unused));
> > +static void trim(char_predicate_t predicate, const char *input, char 
> > **output)
> > +{
> > +    char *p, *q, *tmp;
> 
> Ideally you should check input != NULL before dereferencing it. Or you
> need to document this function expects a valid pointer.

(Thanks for looking at this, Wei, but:)

I disagree.  I think in general a function that takes any kind of
pointer should be assumed to require a non-NULL pointer.  It should
not carry out any kind of nullness check; it should just dereference
the pointer (and consequently crash if it is NULL).

If a function permits callers to pass NULL, that should be documented
in the function's doc comment (along, presumably, with the semantics,
if that isn't obvious).

> You didn't restrict number of entries in this function. So a malformed
> "a=b=c" string ends up with "a" in key and "b=c" in value. Is that a
> problem?

That seems desirable to me.  Otherwise `=' becomes another character
that can't be specified in a value.

Ian.

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