klibc - Nov 2020 - [[PATCH v1 0/3] Fix clang build warnings

New clangs emit some warnings. The code isn't wrong, but should be updated
to
prevent warning creep.

Bill Wendling (3):
  [klibc] dash: shell: Fix clang warnings
  [klibc] dash: shell: Fix clang warnings about format string
  [klibc] Kbuild: use an enum to silence a clang warning

 usr/dash/eval.c                | 6 +++---
 usr/dash/jobs.c                | 2 +-
 usr/kinit/nfsmount/dummypmap.c | 3 ++-
 3 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.29.2.222.g5d2a92d10f8-goog

A couple of warnings from clang about old-style field designators and a
logical "not" used in an unconventional way and needs parentheses.

-----------------------------------------------------------------------
usr/dash/eval.c:106:2: warning: use of GNU old-style field designator extension
[-Wgnu-designator]
        name: nullstr,
        ^~~~~
        .name usr/dash/eval.c:107:2: warning: use of GNU old-style field
designator extension [-Wgnu-designator]
        builtin: bltincmd
        ^~~~~~~~
        .builtin usr/dash/eval.c:277:7: warning: logical not is only applied to
the left hand side of this comparison [-Wlogical-not-parentheses]
                if (!exitstatus == isor)
                    ^           ~~
usr/dash/eval.c:277:7: note: add parentheses after the '!' to evaluate
the comparison first
                if (!exitstatus == isor)
                    ^
                     (                 )
usr/dash/eval.c:277:7: note: add parentheses around left hand side expression to
silence this warning
                if (!exitstatus == isor)
                    ^
                    (          )
-----------------------------------------------------------------------

Signed-off-by: Bill Wendling <morbo at google.com>
---
 usr/dash/eval.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/usr/dash/eval.c b/usr/dash/eval.c
index dd144948..145e0b46 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;
-- 
2.29.2.222.g5d2a92d10f8-goog

Build with clang results in some warnings about using a non-string
literal for a format string.

-----------------------------------------------------------------------
usr/dash/jobs.c:429:23: warning: format string is not a string literal
(potentially insecure) [-Wformat-security]
                col = fmtstr(s, 32, strsignal(st));
                                    ^~~~~~~~~~~~~
usr/dash/jobs.c:429:23: note: treat the string as an argument to avoid this
                col = fmtstr(s, 32, strsignal(st));
                                    ^
                                    "%s",
-----------------------------------------------------------------------

Signed-off-by: Bill Wendling <morbo at google.com>
---
 usr/dash/jobs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/usr/dash/jobs.c b/usr/dash/jobs.c
index b9ff1402..f027cc10 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)");
-- 
2.29.2.222.g5d2a92d10f8-goog

Clang emits a warning when constant forlding variable length arrays:

    usr/kinit/nfsmount/dummypmap.c:156:8: warning: variable length array folded
to constant array as an extension [-Werror,-Wgnu-folding-constant]
                char payload[MAX_UDP_PACKET + offsetof(struct rpc_header, udp)];
                     ^
    1 error generated.

Using an enum bypasses the constant folding issue.

Signed-off-by: Bill Wendling <morbo at google.com>
---
 usr/kinit/nfsmount/dummypmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/usr/kinit/nfsmount/dummypmap.c b/usr/kinit/nfsmount/dummypmap.c
index a4e80147..07210c56 100644
--- a/usr/kinit/nfsmount/dummypmap.c
+++ b/usr/kinit/nfsmount/dummypmap.c
@@ -148,12 +148,13 @@ static int check_vrf(struct rpc_auth *vrf)
 
 static int dummy_portmap(int sock, FILE *portmap_file)
 {
+	enum { PAYLOAD_SIZE = MAX_UDP_PACKET + offsetof(struct rpc_header, udp) };
 	struct sockaddr_in sin;
 	int pktlen, addrlen;
 	union {
 		struct rpc_call rpc;
 		/* Max UDP packet size + unused TCP fragment size */
-		char payload[MAX_UDP_PACKET + offsetof(struct rpc_header, udp)];
+		char payload[PAYLOAD_SIZE];
 	} pkt;
 	struct rpc_call *rpc = &pkt.rpc;
 	struct rpc_auth *cred;
-- 
2.29.2.222.g5d2a92d10f8-goog

On Mon, 2020-11-09 at 14:13 -0800, Bill Wendling wrote:
> New clangs emit some warnings. The code isn't wrong, but should be
updated to
> prevent warning creep.
> 
> Bill Wendling (3):
> ? [klibc] dash: shell: Fix clang warnings
> ? [klibc] dash: shell: Fix clang warnings about format string
> ? [klibc] Kbuild: use an enum to silence a clang warning
I've been resisting doing these minor fixes to dash in the hope of
doing a major upgrade, but since I still haven't dared to do that I'm
going to give in.

All applied, thanks.

Ben.
> ?usr/dash/eval.c??????????????? | 6 +++---
> ?usr/dash/jobs.c??????????????? | 2 +-
> ?usr/kinit/nfsmount/dummypmap.c | 3 ++-
> ?3 files changed, 6 insertions(+), 5 deletions(-)
> 
-- 
Ben Hutchings
Design a system any fool can use, and only a fool will want to use it.
-------------- 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/20201212/e3cc9c28/attachment.sig>