Re: [Qemu-devel] [RFC 02/15] qobject: allow NULL for qstring_get_str()

[Peter Xu]

On Tue, Sep 19, 2017 at 03:48:35PM -0500, Eric Blake wrote:
> On 09/14/2017 02:50 AM, Peter Xu wrote:
> > Then I can get NULL rather than crash when calling things like:
> > 
> >   qstring_get_str(qobject_to_qstring(object));
> > 
> > when key does not exist.
> 
> Right now, qdict_get_str() is documented as:
> 
>  * This function assumes that 'key' exists and it stores a
>  * QString object.
> 
> Your code changes that, by making it now return NULL instead of crashing
> on what used to be usage in violation of the contract; compared to
> qdict_get_try_str() which gracefully returns NULL, but which could use
> your new semantics for doing so in fewer lines of code.
> 
> I'm not necessarily opposed to the change, but I worry that it has
> subtle ramifications that we haven't thought about, as well as
> consistency with the rest of the QObject APIs.  It may be better to just
> introduce qstring_get_try_str(), which gracefully handles NULL input,
> and leave the existing function alone; and if you do introduce a new
> helper, it may be worth converting existing clients (perhaps with help
> from Coccinelle) to take advantage of the helper.

I'll take your suggestion.

Though I didn't see much existing codes that can use the new
qstring_get_try_str().  One I found is object_property_get_str().
I can add one more patch to convert its usage.

Thanks,

> 
> > +++ b/qobject/qstring.c
> > @@ -125,7 +125,7 @@ QString *qobject_to_qstring(const QObject *obj)
> >   */
> >  const char *qstring_get_str(const QString *qstring)
> >  {
> > -    return qstring->string;
> > +    return qstring ? qstring->string : NULL;
> >  }
> >  
> >  /**
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

-- 
Peter Xu