Re: [Qemu-devel] using fdt_setprop() to set properties to empty values

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]

On Fri, Feb 24, 2017 at 09:49:11AM +1100, David Gibson wrote:
> On Thu, Feb 23, 2017 at 08:52:01AM -0600, Eric Blake wrote:
> > On 02/23/2017 06:33 AM, Peter Maydell wrote:
> > > What's the right way to use libfdt's fdt_setprop to set a property
> > > to have an empty value? At the moment in QEMU we tend to use
> > >   fdt_setprop(fdt, nodeoffset, "propertyname", NULL, 0);
> 
> That's what I've always used..
> 
> > > and git grep 'fdt_setprop.*NULL' produces examples of this usage in
> > > PPC and ARM fdt creation code.
> > > 
> > > However the fdt_setprop() documentation doesn't document that a NULL
> > > value pointer is OK if the length is 0, and indeed the implementation
> > > unconditionally calls memcpy(prop->data, val, len), which is
> > > undefined behaviour, and warned about by clang sanitizers if you
> > > build libfdt with them:
> > >  dtc/libfdt/fdt_rw.c:288:21: runtime error: null pointer passed
> > >    as argument 2, which is declared to never be null
> 
> ..but indeed that isn't really technically right.
> 
> > > So what's the best thing to do here? I can't offhand think of a
> > > non-ugly/non-confusing way to pass a valid pointer here...
> > 
> > Does fdt_setprop(fdt, nodeoffset, "propertyname", "", 0) do the right thing?
> 
> That should work, but it's a bit clunky.
> 
> I guess I should add an fdt_setprop_empty() to libfdt.

Ok, I've pushed libfdt upstream patches to (a) make passing NULL to
setprop() with zero length explicitly safe and (b) add an
fdt_setprop_empty() helper macro.  Do you want me to make a pullreq to
update the qemu submodule?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]