Re: [RFC2 RESEND PATCH] [AARCH64] ILP32: support stat syscall family

[Yury Norov <ynorov at caviumnetworks dot com>]

On Wed, Jul 20, 2016 at 04:11:17PM +0000, Joseph Myers wrote:
> On Wed, 6 Jul 2016, Yury Norov wrote:
> 
> >  - I didn't move DECLARE_TIMESPEC() to non-installed file because it's 
> > used in stat.h,
> >    and should be known on used side. If it's wrong please hint me how to 
> >    do this trick.
> 
> First: why is it used in an installed header at all?  The public API uses 
> struct timespec, not struct __timespec.  If you need to reinterpret the 
> bits of struct timespec in some other way, because the kernel ABI is 
> different from the userspace ABI, that is purely a matter for the relevant 
> function implementations and should not affect the installed header.
 
When I wrote it initially, it was an attempt to start solving Y2038
problem, and it looks like this to me:
 - we declare new structure (kernel_timespec or __timespec or timespec64)
   that is identical with 64-bit timespec;
 - we declare struct timespec in existing structures in union with new
   timespec64 (in this patch - __timespec), and use convert_timespec()
   macro where needed;
 - we deprecate struct timespec for new interfaces, and force users to
   use Y2038-safe version.

With all mentioned above, DECLARE_TIMESPEC() and struct __timespec are
both should be available to user in installed header. I was discussing
it with Arnd in comments to previous version to this patch. This is not
needed for arm64/ilp32 port directly, but we'd solve Y2038 somehow.

If there are better idea, I can try it with this port, of course, and then
I will hide 64-bit timespec, and use alignements instead of unions in
public interfaces, as you suggested.

> Second: any interface in an installed header that does not start with 
> _[_A-Z] is a public interface.  This means that (a) it must be permitted 
> to appear in that header by the relevant standards, depending on the 
> feature test macros in use, (b) we must have made a deliberate decision as 
> a community that this is an interface that we wish to support for our 
> users, indefinitely, (c) it must normally be documented and have testsuite 
> coverage, and (d) it must be system-independent unless there is a definite 
> reason for a system-specific interface.  So any macros, typedefs etc. in 
> installed headers that do not meet all those requirements must be in the 
> implementation namespace _[_A-Z]*.  This applies to all such macros added 
> to installed headers by this patch.  That includes statfs_word_t; although 
> POSIX reserves *_t, we should not have any such types in public headers, 
> outside the _[_A-Z]* namespace, unless they are documented public 
> interfaces meeting all of (a)-(d), so you should not use statfs_word_t as 
> a type name.

Thank you. I'll use underscored type / variable names for internal
purposes.

> > +/* Declare structure field that has different size
> > +   in 32- and 64-bit ABIs with paddings where needed,
> > +   so final layout becomes identical.  */
> > +#if __WORDSIZE == 32
> > +# if __BYTE_ORDER == __LITTLE_ENDIAN
> > +#  define __type3264(type, name) type name; type name##_pad
> > +# else
> > +#  define __type3264(type, name) type name##_pad; type name
> > +# endif
> 
> I don't think you want name##_pad; if name is a public interface, then 
> this would make name##_pad a public interface as well, with all the 
> corresponding requirements.  __##name##_pad would be better.  And why 
> isn't this using alignment attributes as in 
> sysdeps/unix/sysv/linux/generic/bits/stat.h?  I wonder if this needs to be 
> separate from __field64 there or if there could be some common macro?

__field64 is more complex and specific macro. I think we can use __type3264
to declare __field64 where needed, I'll do it in next submission.

> -- 
> Joseph S. Myers
> joseph@codesourcery.com