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

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 02/23/2015 12:16 PM, Jan Beulich wrote:
>>>> On 23.02.15 at 10:34, <rcojocaru@bitdefender.com> 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.

>> @@ -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;
>> +
> 
> If we were to go this proposed route, the insertion would need
> to happen above the #define.

Of course, if there are no other objections I'll move it above the
#define and submit V2.


Thanks,
Razvan