[PATCH 2/3] msqid: Provide internal copy of struct __msqid64_ds

Adhemerval Zanella adhemerval.zanella@linaro.org
Wed Nov 11 18:14:47 GMT 2020



On 09/11/2020 21:02, Lukasz Majewski wrote:
> Hi Joseph, Adhemerval,
> 
>> On Mon, 9 Nov 2020, Lukasz Majewski wrote:
>>
>>>> If in a particular case defining the struct tag as a macro is
>>>> unsafe,  
>>>
>>> Could you explain why having construct as:
>>>
>>> #define timespec __timespec64 in
>>> ./time/bits/types/struct_timespec.c is unsafe (after having the
>>> file ./time/bits/types/struct___timespec64.c exported)?  
>>
>> I think "#define stat __stat64_time64" is unsafe, since people may do 
>> "#undef stat" to get a function rather than a macro (and thereby
>> suppress the macro expansion for the struct tag as well).
>>
>> I'm less sure there would be any problems with "#define timespec 
>> __timespec64".  The POSIX namespace rules are clearer on e.g. what's 
>> reserved with file scope, than on what's reserved as a macro name
>> that the user mustn't undefine.
>>
> 
> Please correct me if I'm wrong, but the scheme shall be consistent.
> Which approach shall be taken then?

As Joseph has put, some header name space allows the use of use 
identifiers prefix by the implementation.  However, there are
restriction as well of what the implementation is allowed to do,
check the 'Name Space' part at the POSIX definition [1].

For instance, for the sys/msg.h interface we can export the
'struct msqid64_ds' type since 'msg' is a reserve prefix. And I
think that doing

    #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
    # define msqid_ds __msqid64_ds
    #endif

For 'sys/msg.h' should be safe as well.

However, doing:

  #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
  # define timespec __timespec64
  #endif

It is not allowed since 'timespec' does not fall on the reserved
namespace of 'time.h' header.   As indicated by Joseph, if in a 
particular case defining the struct tag as a macro is unsafe (as for
timespec), we would need to redefine the struct member itself depending
of the _TIME_BITS.

It means that for timespec, we will need to do something as:

  1. On 'time/bits/types/struct_timespec.h' define 'struct timespec' as:

  struct timespec
  {
  #if __TIMESIZE == 32 && defined __USE_TIME_BITS64
    __tim64_t tv_sec;             /* Seconds.  */
  #else
    __time_t tv_sec;
  #endif
  #if __WORDSIZE == 64 \
    || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \
    || __TIMESIZE == 32
    __syscall_slong_t tv_nsec;    /* Nanoseconds.  */
  #else
  # if __BYTE_ORDER == __BIG_ENDIAN
    int: 32;           /* Padding.  */
    long int tv_nsec;  /* Nanoseconds.  */
  # else
    long int tv_nsec;  /* Nanoseconds.  */
    int: 32;           /* Padding.  */
  # endif
  #endif
  };

  
  2. And for the interfaces that uses it, on time/time.h:

  [...]
  #ifdef __USE_POSIX199309
  [...]
  #if __TIMESIZE == 64 || !defined __USE_TIME_BITS64
  extern int nanosleep (const struct timespec *__requested_time,
                        struct timespec *__remaining);
  #else
  # ifdef __REDIRECT_NTH
  extern int __REDIRECT_NTH (nanosleep (const struct timespec *__requested_time,
				        struct timespec *__remaining),
                                        __nanosleep64);
  # else
  /* This might not be safe, but I don't see a better way to do it without
     __REDIRECT_NTH support.  */
  #  define nanosleep __nanosleep64
  # endif
 

  3. It would be good to add a internal tests to check if the export
     ABI for timespec does in fact match the internal definition.  A test that
     does something:

     #define _TIME_SIZE 64

     _Static_assert (sizeof (struct timespec)
                     == sizeof (struct __timespec64),
                     ...);
  
     _Static_assert (sizeof ((struct timespec){0}.tv_sec)
                     == sizeof ((struct __timespec64){0}.tv_nsec),
                     ...);
     _Static_assert (offsetof (struct timespec, tv_sec)
                     == offsetof (struct __timespec64, tv_sec),
                     ...); 
     _Static_assert (sizeof ((struct timespec){0}.tv_nsec)
                     == sizeof ((struct __timespec64){0}.tv_nsec),
                     ...);
     _Static_assert (offsetof (struct timespec, tv_nsec)
                     == offsetof (struct __timespec64, tv_nsec),
                     ...); 
      
     I don't think it needs test for the internal fields (unless the implementation
     does use it in some way, which I don't think it is the case).

 
  4. For timespec we should change all symbols that uses it, including compound
     types.


  5. We need to make sure glibc does not build or define _TIME_SIZE internally
     (we already have issues if user tries to enable the LFS support, so it should
     be ok as-is with default values).


I think I have outlined what is required, but I might have overlooked something.


[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20201111/35352089/attachment-0001.sig>


More information about the Libc-alpha mailing list