Memory leaks with kbhit for Linux

Question

The code is here. When I run my program (I saved kbhit as a header file and kept it in my program folder), I get an uninitialized read access on the first instance of using kbhit (I am using DrMemory for memory debugging). I included sys/ioctl.h as my program wasn’t able to use FIONREAD without it. The thing that is having an issue is the call to tcsetattr(STDIN, TCSANOW, &term); I don’t fully understand how this works so any help would be appreciated. Thank you!

Edit: the exact message is “UNINITIALIZED READ: reading 12 bytes. system call ioctl.0x5402 parameter #2.” The line is from the tcsetattr() call. This error comes after saving kbhit as a cpp file and templating it in another file. The program runs just fine except for that one error.

Given that `tcsetattr` will get called exactly once for the program's lifetime, it becomes questionable to call it a memory leak, as any allocation it makes will get released when the process exits. The only think I can think of is that you allude to saving "kbhit as a header file". Does that mean you are possibly linking in multiple copies of this code for each source file (and invoking it from different source files). Just to rule out the improbable, make sure you aren't having code in header files. — selbie, Feb 18 '19 at 06:05
I saved the function in a header and include it in the main file since that’s the only place where I use it. — Vivek Verghese, Feb 19 '19 at 07:21
You *are* calling tcgetattr to fill in the struct values *before* calling tcsetattr. Right? — Zan Lynx, Feb 19 '19 at 14:45
Hey, it could also be a bug with DrMemory not knowing that `tcgetattr` writes into the structure. I know valgrind has all kinds of custom rules to define things like that for well-known libraries. — Zan Lynx, Feb 20 '19 at 23:18
@ZanLynx hey I think that using valgrind worked. It might be as you said an issue with DrMemory itself. Thank you for the help! — Vivek Verghese, Feb 27 '19 at 10:13

Zan Lynx · Answer 1 · 2019-02-18T06:24:58.250

Here is a version of the code which I modified to be actual C and not C++, since it was only being C++ out of carelessness with bool true/false and struct keywords.

And oh yeah, don't put this in a header file. Put it in a file called kbhit.c and delete or comment out the test main function. And in a header file just write the line:

int _kbhit(void);

Or you might need:

extern "C" int _kbhit(void);

That's all you need in the header.

/**
 Linux (POSIX) implementation of _kbhit().
 Morgan McGuire, morgan@cs.brown.edu
 */
#include <stdbool.h>
#include <stdio.h>
#include <sys/ioctl.h>
#include <termios.h>
#include <unistd.h>

int _kbhit(void) {
        static bool initialized = false;

        if (! initialized) {
                // Use termios to turn off line buffering
                struct termios term;
                tcgetattr(STDIN_FILENO, &term);
                term.c_lflag &= ~ICANON;
                tcsetattr(STDIN_FILENO, TCSANOW, &term);
                setbuf(stdin, NULL);
                initialized = true;
        }

        int bytesWaiting;
        ioctl(STDIN_FILENO, FIONREAD, &bytesWaiting);
        return bytesWaiting;
}

//////////////////////////////////////////////
//      Simple demo of _kbhit()

int main() {
        printf("Press any key");
        while (! _kbhit()) {
                printf(".");
                fflush(stdout);
                usleep(1000);
        }
        printf("\nDone.\n");

        return 0;
}

It looks correct to me and valgrind does not complain. I don't have Dr. Memory to check with.

How this code works is that it first uses tcgetattr to read the termios (terminal input output settings, I think) struct. Then it modifies it by unsetting the ICANON bits. Canon is the canonical setting for terminals which includes line buffering. Then it writes the new termios values back to the terminal. with tcsetattr.

The ioctl call gets how many bytes are waiting in the buffer. If there's bytes waiting, then someone pressed some kind of keys.

Please see the edit above for more information. I changed it from a header to a cpp file (so I can compile with *.cpp). If needed I can post the entire project. It’s just Snake in C++. I needed a way to read input from the keyboard without interrupting the game so making a kbhit function that works with Linux was necessary. — Vivek Verghese, Feb 19 '19 at 07:34

Memory leaks with kbhit for Linux

1 Answers1