stdio: rename short _fileno to _fileno_short for legacy abi compatibility, add new int _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.

[des]

lib/libc/stdio/fopen.c

63

In FreeBSD, no cast is needed as SHRT_MAX is defined as 0x7fff which has type int.

Some downstream projects which are liable to ingest this code at some point may or may not misguidedly define it as 0x7fffU which could set off warnings about comparing expressions of differing signedness. I assume that is why you added a cast and I agree.

I would prefer casting SHRT_MAX over casting f, but neither option is more correct than the other.

[kris_tranception.com]

lib/libc/stdio/fopen.c

63

The intention of the cast was actually to force an unsigned comparison to catch values less than zero as well as those greater than SHRT_MAX.

Alternatively the lengthier expression f < 0 || f > (signed)SHRT_MAX ? -1 : f could instead be used, with the cast to handle if SHRT_MAX happens to be defined as an unsigned constant.

For fdopen() and vdprintf() the caller passes in the file descriptor and thus it could be arbitrary/byzantine, hence care to ensure _fileno_short is always either -1 or a plausibly valid non-negative file descriptor representable in a short.

[des]

lib/libc/stdio/fopen.c

63

In fopen() and freopen(), if the file descriptor is negative, we error out before reaching this point, so there is no issue.

In fdopen() and vdprintf(), we should immediately return EBADF if the caller-provided descriptor is negative, just like we currently return EMFILE if it exceeds SHRT_MAX.