stdio: rename _file to _fileno in struct __sFILE

[kris_tranception.com]

Summary

Kernel file descriptors are 32-bit integers whereas the _file field in libc stdio FILE is a 16-bit short integer.
As a result libc stdio calls such as fopen(), fdopen(), etc. fail when a process has more than 32,767 file descriptors (through whatever library or syscall opened) in use.

This change is change is in two parts:

• D54354: rename _file to _fileno to break source code that directly attempts to access this internal libc stdio FILE field
• D54355: add a new 32-bit _fileno file at end of struct FILE; retain old 16-bit field but renamed for binary compatibilty

Please see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=291610#c2 for further background and related bug reports.

Test Plan

Have locally rebuilt and installed 16-CURRENT kernel & world, as well as git and emacs-nox ports, which include dependencies perl and gnulib (part of m4). No issues noted, in particular the perl issues reported in bug #203900 ten years ago were not observed (perl now uses fdclose() instead of overwriting _file). gnulib as well as /bin/cat in the base system both access internal FILE members other than _file directly but appeared to work correctly.

The following simple test program below also worked as expected (failed on current stdio, succeeded on patched stdio, binary compiled against current stdio but run on patched stdio succeeded with fileno() reporting the actual file descriptor but fileno_unlocked() (which is implemented as a macro) reporting -1).

#include<stdio.h>
#include<sysexits.h>
#include<err.h>
#include<unistd.h>
#include<sys/socket.h>
int main() {
  int f;
  for (int i=0; i<0x8000; i++)
    if ((f=socket(PF_INET, SOCK_STREAM, 0))<0)
      err(EX_OSFILE, "socket");
  if (close(f)<0)
    err(EX_OSFILE, "close");
  FILE* const fp = fopen("/dev/null", "r");
  if (!fp) err(EX_OSFILE, "fopen");
  printf("fileno         : %d\n", fileno(fp));
  printf("fileno_unlocked: %d\n", fileno_unlocked(fp));
  if (fclose(fp) == EOF)
    err(EX_OSFILE, "fclose");
  return 0;
}

Question: should /usr/src/lib/libc/tests/stdio/fopen_test.c and friends have a test case for >32,767 currently open sockets? Note this could fail for other reasons such as low resource limits on the build machine, sysctl set low, etc.

All ports should be built against this to see what other ports might break.

[kris_tranception.com]

Remove inadvertent edit to comment (i.e. read_only_fileno should have remained file_only_file)

[kevans]

+des for recent stdio work, jhb for having worked on something like this specifically that hadn't landed

[kris_tranception.com]

rebased to freebsd/main

[jhb]

I don't think renaming the field does any good here. This isn't the field that existing software accesses directly, and all the software that does is using fileno() which is already part of the API and exposes the ABI.

[kris_tranception.com]

Renaming _file is merely to identify ports that attempt access _file directly so that the consequences of widening _file on them can be reviewed and appropriate action taken.

In this (decade) old comment, you identified perl as problematically accessing _file directly:
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=172332#c8

In the current ports tree, perl5.38, 5.40 and 5.42 all now use fdclose() so perl is no longer a problem with respect to _file, however there may well be other badly behaved ports.

Outside of intentionally breaking source that attempts to directly access _file, the name change has no other purpose and is not necessary.

[kris_tranception.com]

From the exp-run failure logs here...

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=291610#c5

... both games/xpipeman and games/xrobots access FILE->_file directly, though the offending lines are similar to

flock(scorefile->_file, LOCK_UN);

so _file can (and should) simply be replaced with fileno().

[kris_tranception.com]

I have opened the following report in respect of the games/xpipeman and games/xrobots ports, attaching a patch patching the patch files in the ports such that they call fileno() instead accessing _file directly.

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=292939

As no other ports broke in the exp-run, once these two are fixed it would appear that no ports access _file directly and thus no need to either rename _file or expose it publicly in stdio.h.

[kris_tranception.com]

FYI - I posted the following comment to the bug report related to this; reposting here for those not on the bug report.

Bug #291610, Comment #13

I have reworked the D54354/D54355 work into a nine-commit github pull request detailed below.

This iteration is ABI backwards-compatible, does not change the size or definition of struct FILE and provides versioned definitions of fopen, fdopen and freopen that maintain the previous 16-bit behaviour for existing binaries.

As a debugging and troubleshooting aid, the previous 16-bit behaviour can also be enabled by setting a specific environment variable and/or defining a specific weak symbol.

I kindly ask for this pull request be reviewed.

https://github.com/freebsd/freebsd-src/pull/2005

c209e54a5dae libc/stdio: Edit all files for issues reported by checkstyle9.pl
899f05eccdcd libc/stdio: Support 32-bit file descriptors in FILE
c63b13fe06b6 libc/stdio: Remove fileno macros, add internal fileno get/set functions
a65d3458c0fe libc/stdio: Move _cleanup() from findfp.c to stdio.c
972229566cbe libc/stdio: Initialise stdio at start up instead of during runtime
8553d1b4b977 libc/stdio: Fix file descriptor leak in freopen() when >SHRT_MAX
cb49f372159f libc/stdio: Ensure passed file desciptor open for writing
f1eba0feca3e libc/stdio: Fail with EBADF if passed file descriptor negative
da36ca501cb5 libc/stdio: Fix various FILE initialisation and cleanup issues