Re: termios constants should be unsigned

[enh <enh@google.com>]

On Wed, Jun 12, 2024 at 3:01 PM Alejandro Colomar <alx@kernel.org> wrote:
>
> On Wed, Jun 12, 2024 at 01:47:03PM GMT, enh wrote:
> > hacked these changes into AOSP, and it did break one bit of existing
> > code that was already working around the sign differences --- this
> > warning was enabled but the code had a cast to make the _other_ side
> > of the comparison signed (rather than make this side of the comparison
> > unsigned).
>
> BTW, that seems to be a bogus way to workaround this; the cast should
> have been on the other side.  I'd say whoever maintains that code should
> probably fix that to use unsigned types.

indeed. i've already sent out such a change :-)

>  These constants are meant to
> be 'tcflag_t', so a cast should be to that type, or the type of the
> other side of the comparison, but casting to 'int' just for silencing a
> waring seems nuts.

i suspect the reasoning was one of readability --- keeping the [short]
constants legible at the cost of making the expression slightly
longer.

> This makes me wonder if breaking _those_ users could be a good thing...

like Paul Eggert said somewhere else today --- only if we're finding
real bugs. and so far we're not.

it's like the warn_unused_result argument. a purist would argue that
every function should have that annotation, because you should always
check for errors, and if you're not already doing so, your code is
already broken. whereas a pragmatist would argue that most people are
just going to add the "shut up, compiler" cast (or disable the warning
entirely) if their already-working code suddenly starts spamming
warnings next time they build it.

while my bar for that might not be as high as my bar for ABI breakage,
my source compatibility bar is still pretty high. it would be almost
unethical of me to make app developers do random busywork. i have to
be pretty confident (as with, say, "you just passed an fd > 1024 to an
fd_set function/macro and thus corrupted memory") that their code is
_definitely_ wrong. (and even there, that's going to have to be a
runtime check!)

> --
> <https://www.alejandro-colomar.es/>