On Mon, Aug 3, 2020 at 1:48 AM Waldemar Kozaczuk <jwkozac...@gmail.com>
wrote:
> This patch eliminates one more file - unistd/ttyname.c - that is
> identical with the musl copy except for extra 'memset(buf, 0,
> sizeof(buf));' line
> which does not seem necessary.
This is true. Since we call ttyname_r(), it should take care of the null
termination, ttyname() doesn't need its own code to do that.
> There is already existing unit test that
> verifies ttyname() and ttyname_r() behavior.
>
> This patch also cleans up OSv specific implementation of
> ttyname_r() - removes '\0' from a string and unnecessary return -
> per original review of Rean's patch.
>
Good.
> Finally, this patch effectively makes all files (*.c,*.cc) under
> libc/unistd/ "neutral"
> to future musl upgrades.
>
Yes, I guess by "neutral" you mean that it's OSv-specific.
I think as part of the planned upgrade, one thing we should do is, if we
ever do need to patch musl files,
to save them in a special directory so it's clear they are patched musl
file. Even better would be to make
the patches less invasive (like your macro idea). It could have been even
cleaner to save these patches as
a ".patch" and have the Makefile automatically apply this to musl code -
but these patches will be harder to
edit if we ever need to, so I don't know if you like this idea.
>
> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
> ---
> Makefile | 2 +-
> libc/unistd/ttyname.c | 16 ----------------
> libc/unistd/ttyname_r.c | 3 +--
> 3 files changed, 2 insertions(+), 19 deletions(-)
> delete mode 100644 libc/unistd/ttyname.c
>
> diff --git a/Makefile b/Makefile
> index 62e832c7..cd490a76 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1680,7 +1680,7 @@ libc += unistd/getppid.o
> libc += unistd/getsid.o
> libc += unistd/setsid.o
> libc += unistd/ttyname_r.o
> -libc += unistd/ttyname.o
> +musl += unistd/ttyname.o
> musl += unistd/tcgetpgrp.o
> musl += unistd/tcsetpgrp.o
> musl += unistd/setpgrp.o
> diff --git a/libc/unistd/ttyname.c b/libc/unistd/ttyname.c
> deleted file mode 100644
> index 3fa71b29..00000000
> --- a/libc/unistd/ttyname.c
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -#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
> index 0bdba2b7..0ecf4f1d 100644
> --- a/libc/unistd/ttyname_r.c
> +++ b/libc/unistd/ttyname_r.c
> @@ -10,7 +10,7 @@ int ttyname_r(int fd, char *buf, size_t buflen)
> }
> // OSv doesn't support any virtual terminals or ptys so return
> // the fixed pathname of /dev/console
> - char* ttyname = "/dev/console\0";
> + char* ttyname = "/dev/console";
> size_t len = strlen(ttyname);
> if (!isatty(fd)) {
> return ENOTTY;
> @@ -25,5 +25,4 @@ int ttyname_r(int fd, char *buf, size_t buflen)
> return 0;
> }
> }
> - return 0;
> }
> --
> 2.26.2
>
> --
> 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 osv-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/20200802224819.55935-1-jwkozaczuk%40gmail.com
> .
>
--
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 osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/CANEVyjuo_NYdTfSnKeGS86DRV86gZ%2BgKnCFK7KKHKdX-hbz2OQ%40mail.gmail.com.
