[PATCH] libc: replace unistd/ttyname.c with the musl copy

[Waldemar Kozaczuk]

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. 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.

Finally, this patch effectively makes all files (*.c,*.cc) under libc/unistd/ "neutral"
to future musl upgrades.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@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

[Nadav Har'El]

On Mon, Aug 3, 2020 at 1:48 AM Waldemar Kozaczuk <jwkoz...@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.

--
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+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200802224819.55935-1-jwkozaczuk%40gmail.com.

[Commit Bot]

From: Waldemar Kozaczuk <jwkoz...@gmail.com>
Committer: Nadav Har'El <n...@scylladb.com>
Branch: master

libc: replace unistd/ttyname.c with the musl copy

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. 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.

Finally, this patch effectively makes all files (*.c,*.cc) under libc/unistd/ "neutral"
to future musl upgrades.

Signed-off-by: Waldemar Kozaczuk <jwkoz...@gmail.com>

Message-Id: <20200802224819.5...@gmail.com>

---
diff --git a/Makefile b/Makefile

--- 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

--- a/libc/unistd/ttyname.c
+++ b/libc/unistd/ttyname.c

@@ -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