Re: [Xen-devel] [PATCH v3 07/11] tmem: Make the uint64_t oid[3] a proper structure: tmem_oid

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 08/31/15 5:38 PM >>>
>On Mon, Aug 31, 2015 at 05:38:45AM -0600, Jan Beulich wrote:
>> >>> On 28.08.15 at 20:53, <konrad.wilk@xxxxxxxxxx> wrote:
>> > @@ -2714,7 +2719,7 @@ long do_tmem_op(tmem_cli_op_t uops)
>> >              write_unlock(&tmem_rwlock);
>> >              read_lock(&tmem_rwlock);
>> >  
>> > -            oidp = (struct oid *)&op.u.gen.oid[0];
>> > +            oidp = (struct tmem_oid *)&op.u.gen.oid[0];
>> 
>> AIUI this is going to go away later anyway, but generally I think it
>> would be better to hide explicit casts like this by using container_of()
>> when possible.
>
>I am not sure if it will be possible as the structure that contains
>the tmem_oid is an anonymous structure within an union:
>
>struct tmem_op {
>blah
>union {
>struct {
>tmem_oid_t oid;
>} gen;
>} u;
>}
>
>And the 'container_of' macro looks to require only one level of
>nesting.

I'm pretty sure the macro can deal with both.

>> > --- a/xen/include/public/sysctl.h
>> > +++ b/xen/include/public/sysctl.h
>> > @@ -737,6 +737,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_sysctl_psr_cat_op_t);
>> >  #define XEN_SYSCTL_TMEM_OP_RESTORE_PUT_PAGE       32
>> >  #define XEN_SYSCTL_TMEM_OP_RESTORE_FLUSH_PAGE     33
>> >  
>> > +struct tmem_oid {
>> > +    uint64_t oid[3];
>> > +};
>> > +typedef struct tmem_oid tmem_oid_t;
>> > +DEFINE_XEN_GUEST_HANDLE(tmem_oid_t);
>> 
>> I know this is going to be a boring mechanical thing, but I'd really
>> like to see this to be xen_tmem_oid (and alike), especially since
>> you intend to also use the type for th non-tools part of the
>> interface.
>
>This throws a wrench in the compat autogeneration tool.
>
>I keep on getting:
>
>Fields of 'compat_xen_tmem_oid' not found in 'compat/tmem.h'
>and it failing to generate compat/.xlat/tmem.h file. Sticking an prefix of
>xen to the 'common/compat/tmem_xen.c' or just leaving it as is did not help:

Did you perhaps forget to adjust include/xlat.lst? I'm certain adding a prefix 
won't
break everything, albeit iirc there's not going to be a compat_xen_tmem_oid, but
the xen_ prefix in such cases gets replaced by compat_.

Jan


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