Re: [RFC PATCH] __fxstat: replace if() with #if when checking version

[Zack Weinberg <zackw@panix.com>]

On Tue, Sep 6, 2016 at 8:55 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> __fxstat() is always passed with _STAT_VER as vers parameter and it's
> in internal namespace.
>
> It means we can replace runtime argument check with compile-time #ifdef.
> It helps to make the body of _fxstat() smaller, and suppresses errors
> if struct kernel_stat or __xstat_conv() is not declared, when they
> are not needed.

We can't do this.  I think you may have already figured this out, but
let me write out a clear explanation for the benefit of future people
who may have the same idea.

_STAT_VER exists because the contents of struct stat have changed over
time.  When an old executable is used with a new C library, the 'vers'
argument will indicate that the executable expects the old layout, and
the library will translate from the kernel's (presumably newer) stat
layout to the executable's older expectations.  It is the same problem
that the kernel solves by changing the syscall numbers for
stat/fstat/lstat whenever they change the structure.  Therefore, the
'vers' argument *can* vary at runtime, even though all the code in the
*current* source tree always passes the same value.

To make this work, we have inline functions in <sys/stat.h>, backed up
by out-of-line copies in libc_nonshared.a, that present the documented
API (stat/fstat/lstat) and call __xstat/__fxstat/__lxstat with the
additional argument.  That is why these __-name functions are part of
the public ABI for libc.so.  Same principle as __printf_chk for
instance.

If we were designing this mechanism today from scratch we would
probably use symbol versions - unless there's some nonobvious reason
why that wouldn't work - but this mechanism *predates* symbol
versioning.  It has been around since the 1990s if not earlier.  Maybe
you could look into arranging to use symbol versions from now (or
better, the next time the kernel changes the stat structure) going
forward.  But the old __(|f|l)xstat symbols need to stick around for
binary compatibility, and their implementations need to test their
'vers' argument at runtime.

zw