[RFC PATCH] [AARCH64] ILP32: support stat syscall family

[Yury Norov]

On Thu, Jun 30, 2016 at 01:51:22PM +0000, Joseph Myers wrote:
> On Thu, 30 Jun 2016, Yury Norov wrote:
> 
> >  - __32_BIT_ABI_SUPPORTS_64_BIT_TIME_T macro introduced to indicate that 32-bit
> >    ABI has struct __timespec (and maybe __timeval in future) that is compatible
> >    to 64-bit _timespec;
> 
> You can't add a macro to one architecture like that then use it in generic 
> code.  You have to define it in *every* architecture's bits/typesizes.h 
> before it can be used in generic code.

Is it OK to do like this then?
#define __32_BIT_ABI_SUPPORTS_64_BIT_TIME_T /* w/o 1 or 0 */

Where's a proper place for it? In features.h maybe?

> > +      return INLINE_SYSCALL (fstatfs64, 2, fd, buf);
> 
> Bad indentation.  Likewise elsewhere in this patch.
> 
> > +++ b/sysdeps/unix/sysv/linux/aarch64/ilp32/statfs64.c
> > @@ -0,0 +1,37 @@
> > +/* Copyright (C) 2011-2015 Free Software Foundation, Inc.
> 
> Still missing descriptive first line and 2016 in copyright dates.
> 
> > +   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
> 
> There should be no "Contributed by" lines in new files.
> 
> > +  return result;
> > +#endif
> > +#if __32_BIT_ABI_SUPPORTS_64_BIT_TIME_T
> > +      if (!result)
> > +        {
> > +	   conv_timespec(&buf->st_atim, &buf->__st_atim);
> > +	   conv_timespec(&buf->st_mtim, &buf->__st_mtim);
> > +	   conv_timespec(&buf->st_ctim, &buf->__st_ctim);
> > +        }
> >  #endif
> 
> Indentation way off.

It looks ugly only in email. If you apply patch, it will be better.

> 
> > +       conv_timespec(&st->st_atim, &st->__st_atim);
> > +       conv_timespec(&st->st_mtim, &st->__st_mtim);
> > +       conv_timespec(&st->st_ctim, &st->__st_ctim);
> 
> Missing space before '('.

This rule is scary, for sure. I cannot autoreplace all such places as
it doesn't work for definitions, and I cannot drop whitespaces everywhere
because it breaks rules. So I have to pay attention to it again and
again. Script you pointed me doesn't understand difference between
definition and usage of macro. I spent 2 hours fixing the fixes I made
following the report of that script.

Could someone give me clear understanding, how exactly this rule
improves readability and helps finding bugs. I write it because this
rule makes real troubles to me, and helps with nothing.

> > +    DECLARE_TIMESPEC (st_atim);		/* Time of last access.  */
> > +    DECLARE_TIMESPEC (st_mtim);		/* Time of last modification.  */
> > +    DECLARE_TIMESPEC (st_ctim);		/* Time of last status change.  */
> 
> DECLARE_TIMESPEC is in the user's namespace, not valid in installed 
> headers.
> 
> > +#  define conv_timespec(ts, _ts)		\
> 
> Again, not valid in installed headers.


What the proper place you suggest? New timer-related private header?

Yury