TCSANOW not defined correctly in unified headers

[alexcohn]

Description

Both platforms/android-14/arch-arm/usr/include/termios.h and platforms/android-14/arch-x86/usr/include/termios.h redefine TCSANOW to TCSETS, which is 0x5402. But for uniform headers, it is only redefined for MIPS.

As a result, tcsetattr(fd, TCSANOW, &cfg) fails with EINVAL on ARM with APP_PLATFORM=android-14, see https://stackoverflow.com/questions/44870554/ndk-15-breaks-serial-port-library.

I believe this will not work even for android-21 and higher, where tcsetattr is part of libc, but I don't have a proof.

Environment Details

• NDK Version: 15.1.4119039.
• ABI: arm, x86
• NDK API level: 14

[enh]

ugh.

when i wrote tcsetattr for the platform in L (21), i wrote this:

int tcsetattr(int fd, int optional_actions, const termios* s) {
  int cmd;
  switch (optional_actions) {
    case TCSANOW: cmd = TCSETS; break;
    case TCSADRAIN: cmd = TCSETSW; break;
    case TCSAFLUSH: cmd = TCSETSF; break;
    default: errno = EINVAL; return -1;
  }
  return ioctl(fd, cmd, s);
}

but apparently the NDK had this instead:

#undef  TCSANOW
#define TCSANOW    TCSETS

#undef  TCSADRAIN
#define TCSADRAIN  TCSETSW

#undef  TCSAFLUSH
#define TCSAFLUSH  TCSETSF

static __inline int tcsetattr(int fd, int __opt, const struct termios *s) {
  return ioctl(fd, __opt, (void *)s);
}

only half of which was been cleaned up when we added legacy_termios_inlines.h.

this bug gives me such deja vu that i'm sure i've been through legacy_termios_inlines.h at some point, but git log says otherwise. (there's a mysterious reference to "getting bogged down yesterday" in the code review for danalbert's latest change to that file, so i'm not completely mad, but it seems like i gave up for unknown reasons. there also doesn't seem to be an internal bug filed, so it looks like we just dropped the ball here.)

[fornwall]

So when targeting android-21 or later with unified headers, so that the inline versions in legacy_termios_inlines.h are skippped, tcsetattr(fd, TCSANOW, &cfg) will actually currently:

1. Work on non-mips, since TCSANOW is not redefined and passed into the libc implementation given above which expects the original value (0).

2. Not work on mips, since TCSANOW gets redefined which the libc function does not expect.

And when targeting earlier versions than android-21, the situation is reversed and only work on mips ?

[alexcohn]

@fornwall look likely, but I must confess that I never tried any of these in real life.

[enh]

no, it'll work fine for all architectures [>=21] because the implementation is compiled against those headers, so the implementation will be looking for the implementation-specific value of TCSANOW, whatever that is.

[enh]

proposed fix: https://android-review.googlesource.com/429041

it's a little bit ugly, which is probably where i ground to a halt last time, but having two different implementations is [demonstrably] worse.