Thanks! re: the unnecessary explicit null You're right.
re: why the compiler does not complain about that final return statement No clue :) thanks, Rean On Mon, Dec 12, 2016 at 2:02 AM, Nadav Har'El <[email protected]> wrote: > Thanks. I committed the patch. Two notes below. > > > -- > Nadav Har'El > [email protected] > > On Mon, Dec 12, 2016 at 9:24 AM, 'rean' via OSv Development < > [email protected]> wrote: > >> --- >> Makefile | 2 ++ >> libc/unistd/ttyname.c | 16 ++++++++++++++++ >> libc/unistd/ttyname_r.c | 29 +++++++++++++++++++++++++++++ >> modules/tests/Makefile | 3 ++- >> tests/tst-ttyname.c | 49 ++++++++++++++++++++++++++++++ >> +++++++++++++++++++ >> 5 files changed, 98 insertions(+), 1 deletion(-) >> create mode 100644 libc/unistd/ttyname.c >> create mode 100644 libc/unistd/ttyname_r.c >> create mode 100644 tests/tst-ttyname.c >> >> diff --git a/Makefile b/Makefile >> index 9182618..3ef3b54 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1652,6 +1652,8 @@ libc += unistd/getpgrp.o >> libc += unistd/getppid.o >> libc += unistd/getsid.o >> libc += unistd/setsid.o >> +libc += unistd/ttyname_r.o >> +libc += unistd/ttyname.o >> >> musl += regex/fnmatch.o >> musl += regex/glob.o >> diff --git a/libc/unistd/ttyname.c b/libc/unistd/ttyname.c >> new file mode 100644 >> index 0000000..3fa71b2 >> --- /dev/null >> +++ b/libc/unistd/ttyname.c >> @@ -0,0 +1,16 @@ >> +#include <unistd.h> >> +#include <errno.h> >> +#include <limits.h> >> +#include <memory.h> >> + >> +char* ttyname(int fd) >> +{ >> + static char buf[TTY_NAME_MAX]; >> + memset(buf, 0, sizeof(buf)); >> + int result; >> + if ((result = ttyname_r(fd, buf, sizeof(buf)))) { >> + errno = result; >> + return NULL; >> + } >> + return buf; >> +} >> diff --git a/libc/unistd/ttyname_r.c b/libc/unistd/ttyname_r.c >> new file mode 100644 >> index 0000000..0bdba2b >> --- /dev/null >> +++ b/libc/unistd/ttyname_r.c >> @@ -0,0 +1,29 @@ >> +#include <unistd.h> >> +#include <errno.h> >> +#include <stdio.h> >> +#include <string.h> >> + >> +int ttyname_r(int fd, char *buf, size_t buflen) >> +{ >> + if (fd < 0) { >> + return EBADF; >> + } >> + // OSv doesn't support any virtual terminals or ptys so return >> + // the fixed pathname of /dev/console >> + char* ttyname = "/dev/console\0"; >> > > The null here is unnecessary - in C, string constants are always > terminated by a null. > > >> + size_t len = strlen(ttyname); >> + if (!isatty(fd)) { >> + return ENOTTY; >> + } else { >> + // Size must be large enough to hold the pathname + NULL char. buf >> must >> + // also not be NULL >> + if (buflen < len + 1 || !buf) { >> + return ERANGE; >> + } else { >> + memcpy(buf, ttyname, len); >> + buf[len] = 0; >> + return 0; >> + } >> + } >> + return 0; >> > > As far as I can tell, there is no way to reach this return 0? > I wonder the compiler doesn't complain. > > >> +} >> diff --git a/modules/tests/Makefile b/modules/tests/Makefile >> index 6e3aea7..3f9cb59 100644 >> --- a/modules/tests/Makefile >> +++ b/modules/tests/Makefile >> @@ -84,7 +84,8 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so >> tst-bsd-evh.so \ >> tst-namespace.so tst-without-namespace.so payload-env.so \ >> payload-merge-env.so misc-execve.so misc-execve-payload.so >> misc-mutex2.so \ >> tst-pthread-setcancelstate.so tst-syscall.so tst-pin.so >> tst-run.so \ >> - tst-ifaddrs.so tst-pthread-affinity-inherit.so >> tst-sem-timed-wait.so >> + tst-ifaddrs.so tst-pthread-affinity-inherit.so >> tst-sem-timed-wait.so \ >> + tst-ttyname.so >> >> # libstatic-thread-variable.so tst-static-thread-variable.so \ >> >> diff --git a/tests/tst-ttyname.c b/tests/tst-ttyname.c >> new file mode 100644 >> index 0000000..ee2efca >> --- /dev/null >> +++ b/tests/tst-ttyname.c >> @@ -0,0 +1,49 @@ >> +#include <stdio.h> >> +#include <unistd.h> >> +#include <memory.h> >> +#include <errno.h> >> +#include <stdbool.h> >> + >> +unsigned int tests_total = 0, tests_failed = 0; >> + >> +void report(const char* name, bool passed) >> +{ >> + static const char* status[] = {"FAIL", "PASS"}; >> + printf("%s: %s\n", status[passed], name); >> + tests_total += 1; >> + tests_failed += !passed; >> +} >> + >> +int main(void) >> +{ >> + printf("Starting ttyname_r/ttyname test\n"); >> + // Basic flow for test >> + // 1) Take fds for stdin, stdout and stderr and call ttyname_r and >> ttyname >> + // 2) Use an invalid fd (-1) and call ttyname_r and ttyname >> + >> + char buf[256]; >> + memset(buf, 0, sizeof(buf)); >> + int fds[4] = {-1, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO}; >> + >> + for (int i = 0; i < 4; i++) { >> + int retval = ttyname_r(fds[i], buf, sizeof(buf)); >> + printf("fd: %d ttyname_r retval: %d, buf: %s\n", fds[i], retval, >> buf); >> + if (fds[i] < 0) { >> + report("[ttyname_r] bad fds\0", retval == EBADF); >> + } else { >> + report("[ttyname_r] std* fd\0", >> + retval == 0 && strcmp(buf, "/dev/console\0") == 0); >> + } >> + char* tty = ttyname(fds[i]); >> + printf("fd: %d ttyname retval: %s\n", fds[i], tty); >> + if (fds[i] < 0) { >> + report("[ttyname] bad fds\0", tty == NULL); >> + } else { >> + report("[ttyname] std* fds\0", strcmp(buf, "/dev/console\0") == >> 0); >> + } >> + memset(buf, 0, sizeof(buf)); >> + } >> + >> + printf("SUMMARY: %u tests / %u failures\n", tests_total, >> tests_failed); >> + return 0; >> +} >> -- >> 2.7.4 >> >> -- >> You received this message because you are subscribed to the Google Groups >> "OSv Development" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> For more options, visit https://groups.google.com/d/optout. >> > > -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
