Implicit conversion warning with own getch function

Question

I found a c implementaion of conio.h's getch(). Sadly it compiles with a comversion warning, and i don't know what i should do to solve it correctly. I found this link, but i don't know how to implement it.

#include<termios.h>
#include<unistd.h>
#include<stdio.h>
#include"getch.h"

/* reads from keypress, doesn't echo */
int getch(void)
{
    struct termios oldattr, newattr;
    int ch;
    tcgetattr( STDIN_FILENO, &oldattr );
    newattr = oldattr;
    newattr.c_lflag &= ~( ICANON | ECHO );
    tcsetattr( STDIN_FILENO, TCSANOW, &newattr );
    ch = getchar();
    tcsetattr( STDIN_FILENO, TCSANOW, &oldattr );
    return ch;
}

/* reads from keypress, echoes */
int getche(void)
{
    struct termios oldattr, newattr;
    int ch;
    tcgetattr( STDIN_FILENO, &oldattr );
    newattr = oldattr;
    newattr.c_lflag &= ~( ICANON );
    tcsetattr( STDIN_FILENO, TCSANOW, &newattr );
    ch = getchar();
    tcsetattr( STDIN_FILENO, TCSANOW, &oldattr );
    return ch;
}

CLANG 3.5 returns:

getch.c:13:24: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
    newattr.c_lflag &= ~( ICANON | ECHO );
                    ~~ ^~~~~~~~~~~~~~~~~~
getch.c:27:24: warning: implicit conversion changes signedness: 'int' to 'unsigned int' [-Wsign-conversion]
    newattr.c_lflag &= ~( ICANON );
                    ~~ ^~~~~~~~~~~
2 warnings generated.

This compiles cleanly under linux with either `gcc` or `clang` with full warnings. It's a pretty standard idiom (i.e. it has worked for quite a while, dating back to Unix/SysV/etc, IIRC). What OS/distro and compiler version is this? Also, what are the exact definitions for c_lflag, ICANON, etc.? — Craig Estey, Jul 04 '16 at 20:41
@CraigEstey Here is one example where the defines are defined as signed: http://www.delorie.com/djgpp/doc/incs/termios.h — 2501, Jul 04 '16 at 20:46
@CraigEstey Debian 8 with Clang3-5 and 3-8 with -pedantic -Wall -Weverything — vortexman100, Jul 04 '16 at 21:07
@vortexman100 I'm not sure -Weverything is meant to be used by users, but including -Wconversion and -Wsign-conversion is a must. — 2501, Jul 04 '16 at 21:12
I don't use `-Wsign-conversion`, 99% of results are false positives and I don't like to mangle my code to avoid bogus warnings — M.M, Jul 04 '16 at 21:14
It's the `-Weverything` (i.e. `-Wsign-conversion`), which is rare [for this]. tcflag_t has been unsigned since ~1990 (along with ICANON as 0000002). So, you can [artificially] add a `0u`. Or, back off on the warnings [which is what I would do]. A _lot_ of [working] code breaks with the extra warning [not just this]. Zillions of programs use `termios` and _very_ few would do the `0u` hack for an overzealous warning. — Craig Estey, Jul 04 '16 at 21:20
Absolutely. I agree with M.M on this. Virtually every time I've seen something flagged, it's been a false positive. I'm usually fine with `-Wall` [sometimes] `-Wextra` in my rules file. Sometimes, I do `-Weverything` manually as a last resort [that's how I've seen the `-Wsign-conversion` warnings]. Of the 1% of the time where I do `-Weverything`, only 1% of that ever pointed me to a bug [and none were sign conversion issues] that saved me a `gdb` session. It's _a_ tool in the bug fix arsenal--YMMV — Craig Estey, Jul 04 '16 at 21:37
I've had bad luck with `-pedantic` [similar false positives]. I didn't mention that before because it had no effect herein. `clang` can be better with merely `-Wall`. It will flag the following, but `gcc` needs `-Wextra`. I found that out the _hard_ way :-) `int x; int y; int main(void) { if (x > 5); y = 7; return y; }` This is from an actual bug I had once [buried in a few hundred lines of recursive code, so `gdb` debug was difficult]. I triple desk checked it but missed it. So, sometimes, it _does_ help to crank up warnings and sift through the false ones to find the one you need. — Craig Estey, Jul 04 '16 at 22:02

2501 · Accepted Answer · 2016-07-04T20:45:00.327

2

Due to integer promotions those defines are promoted to int, but the c_lflag member is an unsigned integer.

Make sure the bitwise operation is done in an unsigned type:

 newattr.c_lflag &= ~( 0u | ICANON | ECHO );
                        ^

edited Jul 04 '16 at 20:45

answered Jul 04 '16 at 20:39

2501

25,460
4
47
87

might need to be `0ul` or `0ull` depending on the width of `ICANON` and `ECHO` – M.M Jul 04 '16 at 21:13
@M.M You shouldn't force a wider type. It gets promoted automatically if necessary. – 2501 Jul 04 '16 at 21:13
ITYM converted; and if the operands are `unsigned int` and `long` (and `long` is longer than `int`) then the result is `long` , not unsigned long – M.M Jul 04 '16 at 21:15
@M.M You're right, but at least you get a new sign-conversion warning if that is the case. – 2501 Jul 04 '16 at 21:18

Implicit conversion warning with own getch function

1 Answers1

Linked