klibc - Mar 2020 - [PATCH v2 5/5] Clean up clang warnings

- Convert the "old style" GNU field designator extension to the new
  style.
- Use proper indexing into a string literal.
- Use "%s" for the format string in "fmtstr".
- Add or remove casts of the "void *" argument of ioctl.
- Convert "if (!exitstatus == isor)" to "if ((!exitstatus) ==
isor)"
  which retains the current semantics, but may not be what the
  programmer intended.

Signed-off-by: Bill Wendling <morbo at google.com>
---
 usr/dash/eval.c          |  8 ++++----
 usr/dash/jobs.c          |  4 ++--
 usr/dash/output.c        | 12 ++++++------
 usr/kinit/initrd.c       |  2 +-
 usr/kinit/ramdisk_load.c |  2 +-
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/usr/dash/eval.c b/usr/dash/eval.c
index ae83508ba160..2fa1a59995da 100644
--- a/usr/dash/eval.c
+++ b/usr/dash/eval.c
@@ -103,8 +103,8 @@ STATIC int bltincmd(int, char **);
 
 
 STATIC const struct builtincmd bltin = {
-	name: nullstr,
-	builtin: bltincmd
+	.name = nullstr,
+	.builtin = bltincmd
 };
 
 
@@ -274,7 +274,7 @@ checkexit:
 			n->nbinary.ch1,
 			(flags | ((isor >> 1) - 1)) & EV_TESTED
 		);
-		if (!exitstatus == isor)
+		if ((!exitstatus) == isor)
 			break;
 		if (!evalskip) {
 			n = n->nbinary.ch2;
@@ -1081,7 +1081,7 @@ eprintlist(struct output *out, struct strlist *sp, int
sep)
 	while (sp) {
 		const char *p;
 
-		p = " %s" + (1 - sep);
+		p = &" %s"[1 - sep];
 		sep |= 1;
 		outfmt(out, p, sp->text);
 		sp = sp->next;
diff --git a/usr/dash/jobs.c b/usr/dash/jobs.c
index 009bbfeee47e..776983b708d6 100644
--- a/usr/dash/jobs.c
+++ b/usr/dash/jobs.c
@@ -426,7 +426,7 @@ sprint_status(char *s, int status, int sigonly)
 				goto out;
 #endif
 		}
-		col = fmtstr(s, 32, strsignal(st));
+		col = fmtstr(s, 32, "%s", strsignal(st));
 #ifdef WCOREDUMP
 		if (WCOREDUMP(status)) {
 			col += fmtstr(s + col, 16, " (core dumped)");
@@ -1394,7 +1394,7 @@ cmdputs(const char *s)
 				str = "${";
 			goto dostr;
 		case CTLENDVAR:
-			str = "\"}" + !(quoted & 1);
+			str = &"\"}"[!(quoted & 1)];
 			quoted >>= 1;
 			subtype = 0;
 			goto dostr;
diff --git a/usr/dash/output.c b/usr/dash/output.c
index f62e7eab0b4e..bb7c6ada155d 100644
--- a/usr/dash/output.c
+++ b/usr/dash/output.c
@@ -71,27 +71,27 @@
 
 #ifdef USE_GLIBC_STDIO
 struct output output = {
-	stream: 0, nextc: 0, end: 0, buf: 0, bufsize: 0, fd: 1, flags: 0
+	.stream = 0, .nextc = 0, .end = 0, .buf = 0, .bufsize = 0, .fd = 1, .flags = 0
 };
 struct output errout = {
-	stream: 0, nextc: 0, end: 0, buf: 0, bufsize: 0, fd: 2, flags: 0
+	.stream = 0, .nextc = 0, .end = 0, .buf = 0, .bufsize = 0, .fd = 2, .flags = 0
 }
 #ifdef notyet
 struct output memout = {
-	stream: 0, nextc: 0, end: 0, buf: 0, bufsize: 0, fd: MEM_OUT, flags: 0
+	.stream = 0, .nextc = 0, .end = 0, .buf = 0, .bufsize = 0, .fd = MEM_OUT,
.flags = 0
 };
 #endif
 #else
 struct output output = {
-	nextc: 0, end: 0, buf: 0, bufsize: OUTBUFSIZ, fd: 1, flags: 0
+	.nextc = 0, .end = 0, .buf = 0, .bufsize = OUTBUFSIZ, .fd = 1, .flags = 0
 };
 struct output errout = {
-	nextc: 0, end: 0, buf: 0, bufsize: 0, fd: 2, flags: 0
+	.nextc = 0, .end = 0, .buf = 0, .bufsize = 0, .fd = 2, .flags = 0
 };
 struct output preverrout;
 #ifdef notyet
 struct output memout = {
-	nextc: 0, end: 0, buf: 0, bufsize: 0, fd: MEM_OUT, flags: 0
+	.nextc = 0, .end = 0, .buf = 0, .bufsize = 0, .fd = MEM_OUT, .flags = 0
 };
 #endif
 #endif
diff --git a/usr/kinit/initrd.c b/usr/kinit/initrd.c
index 7eece2ce5488..5833f2f2c01f 100644
--- a/usr/kinit/initrd.c
+++ b/usr/kinit/initrd.c
@@ -153,7 +153,7 @@ static int run_linuxrc(int argc, char *argv[], dev_t
root_dev)
 			int olddev = open(ramdisk_name, O_RDWR);
 			umount2("/old", MNT_DETACH);
 			if (olddev < 0 ||
-			    ioctl(olddev, BLKFLSBUF, (long)0) ||
+			    ioctl(olddev, BLKFLSBUF, 0) ||
 			    close(olddev)) {
 				fprintf(stderr,
 					"%s: Cannot flush initrd contents\n",
diff --git a/usr/kinit/ramdisk_load.c b/usr/kinit/ramdisk_load.c
index f43339c8fc74..e3e15d81f81b 100644
--- a/usr/kinit/ramdisk_load.c
+++ b/usr/kinit/ramdisk_load.c
@@ -34,7 +34,7 @@ static int change_disk(const char *devpath, int rfd, int disk)
 			ioctl(rfd, CDROMEJECT, 0);
 		} else {
 			/* Non-ejectable floppy */
-			ioctl(rfd, FDRESET, FD_RESET_IF_NEEDED);
+			ioctl(rfd, FDRESET, (void *)FD_RESET_IF_NEEDED);
 		}
 	}
 	close(rfd);
-- 
2.26.0.rc2.310.g2932bb562d-goog

On Fri, 2020-03-27 at 15:29 -0700, Bill Wendling wrote:
> - Convert the "old style" GNU field designator extension to the
new
>   style.
> - Use proper indexing into a string literal.
> - Use "%s" for the format string in "fmtstr".
> - Add or remove casts of the "void *" argument of ioctl.
> - Convert "if (!exitstatus == isor)" to "if ((!exitstatus)
== isor)"
>   which retains the current semantics, but may not be what the
>   programmer intended.
[...]

This is all good, but I don't want to diverge from upstream dash more
than necessary.  I want to upgrade to the current version of dash,
which appears to have fixed all these warnings but not in exactly the
same way.  The reason I haven't done so already is that I don't have
regression tests for dash.

I've cherry-picked the two upstream dash changes that will apply
cleanly, so that at least reduces the number of warnings you'll see.

Can you change this to just fix kinit for now?

Ben.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL:
<https://lists.zytor.com/archives/klibc/attachments/20200328/ba07e0d5/attachment.sig>

Ben Hutchings dixit:
>same way.  The reason I haven't done so already is that I don't have
>regression tests for dash.
There?s another shell that works well with klibc, is small enough?
well 164728 bytes over 70492 of dash on i386, so factor 2.3 as big
but still tiny compared to others (another 4172 bytes smaller if
linked with -shared), and this one does have regression tests. It
lacks a printf builtin, but one can be added, for 2769 bytes .text
and 32 bytes .bss more???a bargain ?

I?ve been running my Debian systems with mksh as /bin/sh (well, the
lksh binary, which enables 'set -o posix' when run as sh, uses the
?long? type for arithmetics as POSIX demands, contains workarounds
for underquoting in common maintainer scripts, and ships the printf
builtin) for quite long (since about lenny, Sep 2008), so it?s got
quite some exposure. (It?s also /system/bin/sh on Android, so?)

Drop me a message if there?s actual interest in integrating mksh.

bye,
//mirabilos
-- 
?Cool, /usr/share/doc/mksh/examples/uhr.gz ist ja ein Grund,
mksh auf jedem System zu installieren.?
	-- XTaran auf der OpenRheinRuhr, ganz begeistert
(EN: ?[?]uhr.gz is a reason to install mksh on every system.?)