This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [RFC2 RESEND PATCH] [AARCH64] ILP32: support stat syscall family
- From: Yury Norov <ynorov at caviumnetworks dot com>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: <libc-alpha at sourceware dot org>, <arnd at arndb dot de>, <catalin dot marinas at arm dot com>, <davem at davemloft dot net>, <szabolcs dot nagy at arm dot com>, <maxim dot kuvyrkov at linaro dot org>, <pinskia at gmail dot com>, <bamvor dot zhangjian at huawei dot com>, <schwab at suse dot de>, <fweimer at redhat dot com>, <Prasun dot Kapoor at cavium dot com>, <adhemerval dot zanella at linaro dot org>
- Date: Mon, 1 Aug 2016 18:53:56 +0300
- Subject: Re: [RFC2 RESEND PATCH] [AARCH64] ILP32: support stat syscall family
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=Yuri dot Norov at caviumnetworks dot com;
- References: <1467816908-15290-1-git-send-email-ynorov@caviumnetworks.com> <alpine.DEB.2.20.1607201556470.12251@digraph.polyomino.org.uk>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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