Re: [Xen-devel] [PATCH] xen: Minor modifications to platform.h to make it C++-friendly

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.02.15 at 11:31, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 02/23/2015 12:16 PM, Jan Beulich wrote:
>>>>> On 23.02.15 at 10:34, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> Moved the definition of struct xenpf_efi_guid up, and rearranged
>>> struct xenpf_efi_time in the containing union to avoid compilation
>>> errors with C++ (structs defined inside unnamed structs become
>>> unavailable outside their scope with C++). The change allows C++
>>> applications to use platform.h with no consequences for current
>>> C clients.
>> 
>> What's wrong with simply naming the union/struct, making for a
>> much smaller change?
> 
> The bigger issue is not that the containing struct is unnamed, it is
> that from C++'s standpoint the inner struct needs to be explictly
> qualified, as if it were in a namespace. The fact that the outer struct
> is unnamed makes it impossible to refer to the inner struct, but even if
> it were possible to refer to it (by naming the outer struct), it would
> still not be valid C code (and the change would arguably not be much
> smaller anyway):
> 
> 154 #define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x00000001
> 155         struct get_time_t {
> 156             struct xenpf_efi_time {
> 157                 uint16_t year;
> 158                 uint8_t month;
> 159                 uint8_t day;
> 160                 uint8_t hour;
> 161                 uint8_t min;
> 162                 uint8_t sec;
> 163                 uint32_t ns;
> 164                 int16_t tz;
> 165                 uint8_t daylight;
> 166             } time;
> 167             uint32_t resolution;
> 168             uint32_t accuracy;
> 169         } get_time;
> 170
> 171         struct get_time_t::xenpf_efi_time set_time;
> 172
> 173 #define XEN_EFI_GET_WAKEUP_TIME_ENABLED 0x00000001
> 174 #define XEN_EFI_GET_WAKEUP_TIME_PENDING 0x00000002
> 175         struct get_time_t::xenpf_efi_time get_wakeup_time;
> 176
> 177 #define XEN_EFI_SET_WAKEUP_TIME_ENABLE      0x00000001
> 178 #define XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY 0x00000002
> 179         struct get_time_t::xenpf_efi_time set_wakeup_time;
> 
> This is why I think the proposed solution works best for everyone.

Ah, right. But then ...

>>> @@ -152,24 +159,24 @@ struct xenpf_efi_runtime_call {
>>>      xen_ulong_t status;
>>>      union {
>>>  #define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x00000001
>>> +        struct xenpf_efi_time {
>>> +            uint16_t year;
>>> +            uint8_t month;
>>> +            uint8_t day;
>>> +            uint8_t hour;
>>> +            uint8_t min;
>>> +            uint8_t sec;
>>> +            uint32_t ns;
>>> +            int16_t tz;
>>> +            uint8_t daylight;
>>> +        } set_time;
>>> +

... this should be moved out to file scope too, both for consistency
and to avoid an eventual further adjustment going forward. Otoh
I'm not convinced we need the headers to be C++ ready (nor am
I convinced that there aren't any other obstacles preventing their
unmodified use in C++). Co-maintainers, what do you think?

Jan


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