inetd: only declare and use the mapped-address netconfig under INET6

[seuros]

Summary

In a WITHOUT_INET6 build the only assignment to netid2 is compiled out and
the non-INET6 arm returns early, so netid2 is unconditionally NULL and the
rpcb_set() call guarded by it is dead code. clang does not prove it dead
and reports nbuf2 as uninitialized where it is passed as a const pointer,

No functional change.

MFC after: 1 week
Reported by: clang (-Wuninitialized-const-pointer)
Suggested by: dim
Approved by: ngie (co-mentor)

[adrian]

I'm not sure who to route this to for review. I'll go poke around!

[ngie]

@alfred was the one who touched the line last, but he's been inactive for many years now.
@des or @kevans: this change seems straightforward, but do you have any objections to this change? I ask because Klara has taken an active role at cleaning up portions of the base system lately.

[dim]

How did you produce the warning? I don't see it when building inetd. In any case, I think it would be better to only declare and use nbuf2 and netid2 if INET6 is defined. This way you can avoid the warning, and no unused data is allocated or used.

I.e. something like:

--- a/usr.sbin/inetd/inetd.c
+++ b/usr.sbin/inetd/inetd.c
@@ -1315,11 +1315,13 @@ setsockopt(fd, SOL_SOCKET, opt, (char *)&on, sizeof (on))
         if (sep->se_rpc) {
                u_int i;
                socklen_t len = sep->se_ctrladdr_size;
-               struct netconfig *netid, *netid2 = NULL;
+               struct netconfig *netid;
 #ifdef INET6
+               struct netconfig *netid2 = NULL;
                struct sockaddr_in sock;
+               struct netbuf nbuf2;
 #endif
-               struct netbuf nbuf, nbuf2;
+               struct netbuf nbuf;

                 if (getsockname(sep->se_fd,
                                (struct sockaddr*)&sep->se_ctrladdr, &len) < 0){
@@ -1360,10 +1362,12 @@ setsockopt(fd, SOL_SOCKET, opt, (char *)&on, sizeof (on))
                 for (i = sep->se_rpc_lowvers; i <= sep->se_rpc_highvers; i++) {
                        rpcb_unset(sep->se_rpc_prog, i, netid);
                        rpcb_set(sep->se_rpc_prog, i, netid, &nbuf);
+#ifdef INET6
                        if (netid2) {
                                rpcb_unset(sep->se_rpc_prog, i, netid2);
                                rpcb_set(sep->se_rpc_prog, i, netid2, &nbuf2);
                        }
+#endif
                 }
         }
        if (sep->se_socktype == SOCK_STREAM)

[seuros]

I built the world without ipv6 support . We don't test that in CI.

I will refactoring this diff.

[seuros]

refactor and dedup code

[ngie]

I think the prior approach suggested by @dim to make as minimal of a change as possible makes a lot of sense: there's no real single-maintainer in inetd and the daemon has remained in maintenance mode for many years.

I'll send you a follow up message soon about this :).

[seuros]

revert refactoring

[ngie]

Approved by: ngie (co-mentor)

[kevans]

In D59277#1360135, @ngie wrote:

I think the prior approach suggested by @dim to make as minimal of a change as possible makes a lot of sense: there's no real single-maintainer in inetd and the daemon has remained in maintenance mode for many years.

I have changes locally to start cleaning it up further after my last round in 2019-2020. I had to take a break from it while I tried to understand what went wong with previous attempts to convet it to kqueue(2), and then it fell off my radar.