NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: bin/55170 dhcpcd fails when used instead of rtsol



The following reply was made to PR bin/55170; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: bin/55170 dhcpcd fails when used instead of rtsol
Date: Wed, 15 Apr 2020 11:53:35 +0700

     Date:        Tue, 14 Apr 2020 11:10:02 +0000 (UTC)
     From:        Roy Marples <roy%marples.name@localhost>
     Message-ID:  <20200414111002.4AFB31A924B%mollari.NetBSD.org@localhost>
 
 Roy:
 
   |  This patch should fix both the quiestness and the error.
   |  There is no need to create the control socket in test mode.
 
 The one thing that patch doesn't address, which it could, is the
 error message that is produced when (for whatever reason, which should
 no longer be this one) control_start() does fail.
 
 It has two problems, first, to anyone but you,
 
 	main: control_start: ....
 
 is meaningless, when I first saw that in the PR messages, I assumed
 that some program/script "control_start" had perhaps not been  installed,
 or not installed correctly.
 
 Could it not say something more like
 
 	Unable to init control socket (main): ...
 
 You don't need to see the "control_start" to know which function it
 was that failed, and to almost everyone else that's useless info,
 and the function that called it is secondary info (useful to know,
 but not vital).   If you want a standard form prefix it would be
 
 	dhcpcd: Unable to init control socket (main): ...
 
 
 And secondly, when control_start() detects an error, and is about to
 return failure, it does (something like)
 
 		unlink(sock.sun_path);
 		close(fd);
 		return -1;
 
 where if one of those sys calls fails (the unlink is certainly there,
 the close I am guessing at, don't remember, and did not look again)
 which is entirely possible, then errno gets modified - so instead of
 it reflecting the error from whatever sys call failed that led into
 this path, it instead indicates that the reason the unlink() failed,
 which no-one cares about.
 
 That sequence needs an
 
 		int err = errno;
 
 		errno = err;
 
 wrapper around it.
 
 Without that, the part of the error message shown above as the elipsis
 is useless.
 
 kre
 
 ps: I'm assuming that Kimmo will test the patch and confirm it works.
 
 


Home | Main Index | Thread Index | Old Index