On Sat, Sep 07, 2019 at 08:12:51PM +0300, Andy Shevchenko wrote: > On Sat, Sep 07, 2019 at 09:32:40AM -0700, Dmitry Torokhov wrote: > > On Sat, Sep 07, 2019 at 07:08:19PM +0300, Andy Shevchenko wrote: > > > On Fri, Sep 06, 2019 at 03:26:09PM -0700, Dmitry Torokhov wrote: > > > > > + } else if (src->type == DEV_PROP_REF) { > > > > + /* All reference properties must be arrays */ > > > > + return -EINVAL; > > > > > > Hmm... What about to duplicate pointer under value union and use is_array > > > to > > > distinguish which one to use? Because... > > > > Then we have to special-case copying this entry, similar to the pains we > > are going with the strings. > > I can't see it as a pain. Simple do the same kmemdup() for the case when > is_array = false and DEV_TYPE_REF?
And then you need to make sure it is freed on error paths and when we remove property entries. This requires more checks and code. In contrast we already know how to handle out of line objects of arbitrary size. The only reason we have inline strings is because for shorter strings we save 4/8 bytes. > > By the way, don't we need to update property_entry_{get,set}_pointer()? I do not see these, where are they? > > > > > + .is_array = true, > > > > \ > > > > > > I really don't like this "cheating". > > > > This is not cheating. Any single value can be represented as an array of > > one element. Actually, the only reason we have this "is_array" business > > is because for scalar values and short strings it is much cheaper to > > store single value in-line instead of out of line + pointer, especially > > on 64 bit arches. > > Yes, and this is a lot of benefit! Yes, nobody argues against it. Here however we are dealing with a larger structure. There is absolutely no benefit of trying to separate single value vs array here. > > > If you want we can change is_array into is_inline. > > Nope, is_array is exactly what it tells us about the content. Its functional > load is to distinguish which union (value vs. pointer) we are using. No, it signifies whether the value is stored within property entry or outside. I can fit probably 8 bytes arrays into property entry structure, in which case is_array will definitely not reflect the data type. It is the type-specific accessors that know how to parse and fetch data from properties. Thanks. -- Dmitry