Date: Thu, 8 Dec 2011 22:13:04 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Pawel Jakub Dawidek <pjd@FreeBSD.org> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org, Eitan Adler <eadler@FreeBSD.org> Subject: Re: svn commit: r228342 - head/contrib/tzcode/zic Message-ID: <20111208200957.F1091@besplex.bde.org> In-Reply-To: <20111208073210.GC1678@garage.freebsd.pl> References: <201112080240.pB82ek4Q066638@svn.freebsd.org> <20111208073210.GC1678@garage.freebsd.pl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 8 Dec 2011, Pawel Jakub Dawidek wrote:
> On Thu, Dec 08, 2011 at 02:40:46AM +0000, Eitan Adler wrote:
>> Log:
>> - set progname for use in usage()
>>
>> Modified: head/contrib/tzcode/zic/zdump.c
>> ==============================================================================
>> --- head/contrib/tzcode/zic/zdump.c Thu Dec 8 00:56:23 2011 (r228341)
>> +++ head/contrib/tzcode/zic/zdump.c Thu Dec 8 02:40:46 2011 (r228342)
>> @@ -260,6 +260,7 @@ char * argv[];
>> register struct tm * tmp;
>> register struct tm * newtmp;
>>
>> + progname=argv[0];
>> INITIALIZE(cutlotime);
>> INITIALIZE(cuthitime);
>> #if HAVE_GETTEXT
>
> It will good to to try to upstream it as this is contributed code.
> This change doesn't seem to be consistent with the style of this file.
> They use spaces around =.
Also, if this were not contrib'ed code, then using progname in it would
involve 2 layers of style bugs:
- user-visible layer: in FreeBSD, the program name printed by usage() is
literal and doesn't depend on the pathname with which the program was
invoked. In old versions of zdump in FreeBSD, one of the main changes
was to convert it to FreeBSD style. Old versions were also missing
the uninitialization of progrname, even in the vendor version:
% Index: zdump.c
% ===================================================================
% RCS file: /home/ncvs/src/usr.sbin/zic/zdump.c,v
% retrieving revision 1.1.1.4
% retrieving revision 1.9
% diff -r1.1.1.4 -r1.9
% ...
% 130d132
% < static char * progname;
% 131a134
% > static void usage(void);
% 160d162
% < progname = argv[0];
% 163,164c165
% < (void) printf("%s\n", elsieid);
% < (void) exit(EXIT_SUCCESS);
% ---
% > errx(EXIT_SUCCESS, "%s", elsieid);
% 174,177c175
% < (void) fprintf(stderr,
% < _("%s: usage is %s [ --version ] [ -v ] [ -c cutoff ] zonename ...\n"),
% < argv[0], argv[0]);
% < (void) exit(EXIT_FAILURE);
% ---
% > usage();
% 204,207c202,205
% < (fakeenv[0] = (char *) malloc(longest + 4)) == NULL) {
% < (void) perror(progname);
% < (void) exit(EXIT_FAILURE);
% < }
% ---
% > (fakeenv[0] = (char *) malloc((size_t) (longest +
% > 4))) == NULL)
% > errx(EXIT_FAILURE,
% > _("malloc() failed"));
% 266,270c264,265
% < if (fflush(stdout) || ferror(stdout)) {
% < (void) fprintf(stderr, "%s: ", argv[0]);
% < (void) perror(_("Error writing standard output"));
% < (void) exit(EXIT_FAILURE);
% < }
% ---
% > if (fflush(stdout) || ferror(stdout))
% > errx(EXIT_FAILURE, _("error writing standard output"));
% 277a273,280
% > static void
% > usage(void)
% > {
% > fprintf(stderr,
% > _("usage: zdump [--version] [-v] [-c cutoff] zonename ...\n"));
% > exit(EXIT_FAILURE);
% > }
This change also breaks the vendor style, since it doesn't use an
excessive (void) cast on the fprintf() in usage().
The other changes mostly involve changing perror() and [f]printf() to
errx(). This is a user-visible change if the [f]printf() was to
stdout instead of stderr, and perhaps if errx() gives better formatting
than perror(). The current version uses the unportable errx() instead
of perror() plus exit(), but still uses [f]printf() instead of warnx()
when not exiting. I can't see any vendor version of it using cvs.
- FreeBSD and NetBSD programs never use progname = argv[0] or use
progname explicitly. They use getprogname(3). This is implemented
by setting a variable __progname to argv[0] in crt. The setting may
be changed by setprogname(3). This gives minor inconsistencies which
may be considered features: usage() always prints the literal program
name, while the errx() family uses getprogname() and this prints
argv[0] or whatever setprogname() changed __progname too.
Grepping for progname in /usr/src/*bin shows only a few violations of
the above rules:
% bin/chio/chio.c: "<from ET> <from EU> <to ET> <to EU> [inv]\n", getprogname(), cname);
% bin/chio/chio.c: getprogname(), cname);
% bin/chio/chio.c: getprogname(), cname);
% bin/chio/chio.c: (void) fprintf(stderr, "usage: %s %s\n", getprogname(), cname);
% bin/chio/chio.c: (void) fprintf(stderr, "usage: %s %s\n", getprogname(), cname);
% bin/chio/chio.c: (void) fprintf(stderr, "usage: %s %s <picker>\n", getprogname(), cname);
% bin/chio/chio.c: getprogname(), cname);
% bin/chio/chio.c: getprogname(), cname);
% bin/chio/chio.c: getprogname(), cname);
% bin/chio/chio.c: "<from ET> <from EU>\n", getprogname(), cname);
% bin/chio/chio.c: "arg1 arg2 [arg3 [...]]\n", getprogname());
chio doesn't look like FreeBSD sources. It came from NetBSD.
% bin/ps/ps.c: getprogname(), *argv);
Someone broke ps. It is internally inconsistent: it prints getprogname()
here, and then immediately calls usage() which prints a literal "ps".
% bin/pkill/pkill.c: if (strcmp(getprogname(), "pgrep") == 0) {
% bin/pkill/pkill.c: " [-t tty] [-u euid] pattern ...\n", getprogname(),
pkill can be a link to pgrep. This is the only excuse for using
getprograme() in usage().
__progname is actually initialized to an edited version of argv[0],
with the path prefix stripped. Thus if pkill is copied to ./cat, and
run from any path leading to ./cat, the above bogusly prints that its
name is `cat'. When pkill is invoked under any other name that pgrep,
it defaults to acting as pkill, but it doesn't change its name to pkill
so what it is acting as is hard to tell from its error messages if its
name is not pkill. Before getprogname() existed, usage() would probably
have printed a name that matched its action using a flag set when its
action was determined (acting_as_pgrep ? "pgrep" : "pkill"). Using
getprogname(), the logic should probably be that if the action is
defaulted then the name should be changed to match the defaulted
action. But it would be clearer to default to an error, with a special
usage message. This the existence of getprogname() doesn't simplify
printing of the active program name in usage() for programs whose action
depends on their name.
% bin/echo/echo.c: char *progname = argv[0];
% bin/echo/echo.c: errexit(progname, "malloc");
% bin/echo/echo.c: errexit(progname, "write");
echo has contortions to avoid using err() since that might break sh when
echo is sh's builtin. It should use getprogname() here. IIRC, this
was implemented before gteprogname() existed.
% sbin/newfs/newfs.c: getprogname(),
SOmeone broken newfs. Fixed in my version.
% sbin/fsck_ffs/main.c: getprogname());
% sbin/fsck/fsutil.c: dev, getprogname());
% sbin/fsck/fsck.c: getprogname(), common);
This is probably from NetBSD>
% sbin/dhclient/dhclient.c: extern char *__progname;
% sbin/dhclient/dhclient.c: openlog(__progname, LOG_PID | LOG_NDELAY, DHCPD_LOG_FACILITY);
% sbin/dhclient/dhclient.c: extern char *__progname;
% sbin/dhclient/dhclient.c: fprintf(stderr, "usage: %s [-bdqu] ", __progname);
Chumminess with the implementation.
% sbin/mdmfs/mdmfs.c: if (strcmp(getprogname(), "mount_mfs") == 0 ||
% sbin/mdmfs/mdmfs.c: strcmp(getprogname(), "mfs") == 0) {
% sbin/mdmfs/mdmfs.c:"\tmd-device mount-point\n", getprogname());
Because it is sometimes named mount_mfs, and this is supported (like
pkill/pgrep).
% sbin/hastctl/hastctl.c: getprogname());
% sbin/hastctl/hastctl.c: getprogname());
% sbin/hastctl/hastctl.c: getprogname());
% sbin/hastctl/hastctl.c: getprogname());
Apparently just style bugs.
% sbin/swapon/swapon.c: getprogname(),
% sbin/swapon/swapon.c: getprogname(), which_prog == SWAPOFF ? "remov" : "add",
% sbin/swapon/swapon.c: fprintf(stderr, "usage: %s ", getprogname());
Because it is sometimes named swapoff or swapctl.
% sbin/devfs/rule.c: setprogname("devfs rule");
% sbin/devfs/rule.c: setprogname("devfs ruleset");
Change to a strange program name with spaces in it. This name would be hard
to parse if it is actually used.
% sbin/md5/md5.c: const char *progname;
% sbin/md5/md5.c: const char* progname;
% sbin/md5/md5.c: if ((progname = strrchr(argv[0], '/')) == NULL)
% sbin/md5/md5.c: progname = argv[0];
% sbin/md5/md5.c: progname++;
Home made version of getprogname(). Probably written before the latter
existed.
% sbin/md5/md5.c: if (strcasecmp(Algorithm[digest].progname, progname) == 0)
% sbin/md5/md5.c: fprintf(stderr, "usage: %s [-pqrtx] [-s string] [files ...]\n", alg->progname);
Because it can have various names.
[... 249 more lines]
Got bored here. The above examples are enough to show that the FreeBSD
style is to use getprogname() in usage() if and only if the program can
have various _supported_ names (not unsupported names like a copy of
/bin/rm to ./cat would have). Oops. I chose rm for maximal danger
when it is renamed to a harmless command. But rm is linked to unlink.
Its logic for determining its action and name are slightly broken, as
for pkill/pgrep but more:
- to determine whether it should act like unlink, it uses a home made
getprogname().
- when its name is neither rm nor unlink, it acts like rm
- when its name is unlink, it normally doesn't call usage(), so if there
is an error then its name is reported as being unlink. However, it
calls usage() if it has any option or no args. usage() doesn't use
getprogname(), so it then prints the usage message for rm, including
rm's literal name. But usage() prints the message for unlink too.
usage() always prints both messages. This gives noise more for the
case where the program is just rm.
Related bug: the program name returned by getprogname() is initialized
as if by setprogname(argv[0]). pkill.c depends on this, but this is not
documented in getprogname(3). It is documented that setprogname() sets
the program name to the last component of its arg, but not that
initialization uses either argv[0] or setprogname() on it. The man
page also says that getprogname() manipulates the program name. These
bugs are fixed or moot in NetBSD's man page, but the semantics of
setprogname() are very different in NetBSD (at least in 2005). In NetBSD,
setprogname() is always called from crt (as in FreeBSD), but subsequent
calls have no effect. It is recommended that all programs start with
a call to setprogname() in main(), just for portability to systems that
don't call it automatically. For systems that don't have it at all,
these calls can be implemented in a special library or annulled by a
macro. It's amazing how complicated keeping track of your own name
is when you use a library/bureauocracy that might not exist to do it :-).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111208200957.F1091>