[DTrace-devel] [PATCH 05/17] uapi headers: include <sys/dtrace_types> as needed

[Eugene Loh]

On 8/30/22 21:18, Kris Van Hees wrote:

> On Tue, Aug 30, 2022 at 08:13:47PM -0400, Eugene Loh via DTrace-devel wrote:
>> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
>>
>> But:
>>
>> *)  Should we add tests?  (I'm not convinced we should -- there are too many
>> code paths -- but see next item.)
>>
>> *)  Maybe in addition to the fix one should replace (the following is
>> pseudocode):
>>
>>          #if THISENDIANNESS
>>                  // this endianness
>>          #else
>>                  // that endianness
>>          #endif
>>
>>      with
>>
>>          #if THISENDIANNESS
>>                  // this endianness
>>          #elif THATENDIANNESS
>>                  // that endianness
>>          #else
>>                  error do not compile
>>          #endif
> But sys/dtrace_types.h already contains:
>
> #if __BYTE_ORDER == __LITTLE_ENDIAN
> #define _LITTLE_ENDIAN 1
> #elif __BYTE_ORDER == __BIG_ENDIAN
> #define _BIG_ENDIAN 1
> #else
> #warning Unknown endianness
> #endif

True.  So that file makes sure that __BYTE_ORDER is either 
__LITTLE_ENDIAN or __BIG_ENDIAN.  This means that _LITTLE_ENDIAN or 
_BIG_ENDIAN will be defined.  (Hopefully, the numbers of leading 
underscores are clear in this email.)

Now, what if a file thinks it's using _LITTLE_ENDIAN or _BIG_ENDIAN, but 
it forgets to include that header file?  If it looks for just one macro, 
that macro will be missing and the other value will be assumed.

One solution is to make sure not to omit the header.  ("Just don't make 
mistakes.")  That's the fix here.

I'm just saying that there is a coding practice that helps make sure 
that such mistakes are detected.

> So there is no need to do this once again.  I do not believe that there is any
> expectation that DTrace will ever get ported to a system that has neither
> little nor big endianness.
>
>> On 8/10/22 18:06, Nick Alcock via DTrace-devel wrote:
>>> This is needed for _LITTLE_ENDIAN etc: without it, consumers are liable
>>> to find that they think they're on a big-endian system even when they're
>> Think they're on a BE system?  Well, it depends on how the code is written:
>> the error might induce one to thinking they're on a little-endian machine.
>> Anyhow, the commit message can be written in a way that does not assume
>> which way the #ifdef is written.
>>
>>> not.
>>> ---
>>>    include/dtrace/actions_defines.h | 1 +
>>>    include/dtrace/dof_defines.h     | 1 +
>>>    2 files changed, 2 insertions(+)
>>>
>>> diff --git a/include/dtrace/actions_defines.h b/include/dtrace/actions_defines.h
>>> index dfec34f3ddc2..f161df7cf13f 100644
>>> --- a/include/dtrace/actions_defines.h
>>> +++ b/include/dtrace/actions_defines.h
>>> @@ -13,6 +13,7 @@
>>>    #ifndef _DTRACE_ACTIONS_DEFINES_H
>>>    #define _DTRACE_ACTIONS_DEFINES_H
>>> +#include <sys/dtrace_types.h>
>>>    #include <dtrace/universal.h>
>>>    /*
>>> diff --git a/include/dtrace/dof_defines.h b/include/dtrace/dof_defines.h
>>> index 02d650c22831..818089d1b472 100644
>>> --- a/include/dtrace/dof_defines.h
>>> +++ b/include/dtrace/dof_defines.h
>>> @@ -13,6 +13,7 @@
>>>    #ifndef _DTRACE_DOF_DEFINES_H
>>>    #define _DTRACE_DOF_DEFINES_H
>>> +#include <sys/dtrace_types.h>
>>>    #include <dtrace/universal.h>
>>>    /*
>> _______________________________________________
>> DTrace-devel mailing list
>> DTrace-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel