svn commit: r308443 - head/bin/hostname

[Bruce Evans]

On Tue, 8 Nov 2016, Renato Botelho wrote:

>> On 8 Nov 2016, at 09:36, Marcelo Araujo <araujo at FreeBSD.org> wrote:
>>
>> Log:
>>  Add -d flag that prints domain only.
>>
>>  PR:		212875
>>  Submitted by:	Ben RUBSON <ben.rubson at gmail.com>
>>  Reviewed by:	pi

This has many style bugs.

>> Modified:
>>  head/bin/hostname/hostname.1
>>  head/bin/hostname/hostname.c
>>
>> Modified: head/bin/hostname/hostname.1
>> ==============================================================================
>> --- head/bin/hostname/hostname.1	Tue Nov  8 10:10:55 2016	(r308442)
>> +++ head/bin/hostname/hostname.1	Tue Nov  8 11:36:33 2016	(r308443)
>> @@ -29,7 +29,7 @@
>> .\"	@(#)hostname.1	8.2 (Berkeley) 4/28/95
>> .\" $FreeBSD$
>> .\"
>> -.Dd December 7, 2006
>> +.Dd November 9, 2016
>> .Dt HOSTNAME 1
>> .Os
>> .Sh NAME
>> @@ -37,7 +37,8 @@
>> .Nd set or print name of current host system
>> .Sh SYNOPSIS
>> .Nm
>> -.Op Fl fs
>> +.Op Fl f
>> +.Op Fl s|d
>> .Op Ar name-of-host
>> .Sh DESCRIPTION
>> The
>> @@ -62,6 +63,8 @@ This is the default behavior.
>> .It Fl s
>> Trim off any domain information from the printed
>> name.
>> +.It Fl d
>> +Only print domain information.
>> .El
>> .Sh SEE ALSO
>> .Xr gethostname 3 ,
>>
>> Modified: head/bin/hostname/hostname.c
>> ==============================================================================
>> --- head/bin/hostname/hostname.c	Tue Nov  8 10:10:55 2016	(r308442)
>> +++ head/bin/hostname/hostname.c	Tue Nov  8 11:36:33 2016	(r308443)
>> @@ -54,11 +54,12 @@ static void usage(void) __dead2;
>> int
>> main(int argc, char *argv[])
>> {
>> -	int ch, sflag;
>> +	int ch, sflag, dflag;
>> 	char *p, hostname[MAXHOSTNAMELEN];
>>
>> 	sflag = 0;
>> -	while ((ch = getopt(argc, argv, "fs")) != -1)
>> +	dflag = 0;
>> +	while ((ch = getopt(argc, argv, "fsd")) != -1)
>> 		switch (ch) {
>> 		case 'f':
>> 			/*
>> @@ -70,6 +71,9 @@ main(int argc, char *argv[])
>> 		case 's':
>> 			sflag = 1;
>> 			break;
>> +		case 'd':
>> +			dflag = 1;
>> +			break;
>> 		case '?':
>> 		default:
>> 			usage();
>> @@ -77,7 +81,7 @@ main(int argc, char *argv[])
>> 	argc -= optind;
>> 	argv += optind;
>>
>> -	if (argc > 1)
>> +	if (argc > 1 || (sflag && dflag))
>> 		usage();
>>
>> 	if (*argv) {
>> @@ -90,6 +94,10 @@ main(int argc, char *argv[])
>> 			p = strchr(hostname, '.');
>> 			if (p != NULL)
>> 				*p = '\0';
>> +		} else if (dflag) {
>> +			p = strchr(hostname, '.');
>> +			if (p != NULL)
>> +				strcpy(hostname, ++p);
>> 		}
>> 		(void)printf("%s\n", hostname);
>> 	}
>> @@ -100,6 +108,6 @@ static void
>> usage(void)
>> {
>>
>> -	(void)fprintf(stderr, "usage: hostname [-fs] [name-of-host]\n");
>> +	(void)fprintf(stderr, "usage: hostname [-f] [s|d] [name-of-host]\n");
>
>
> It’s missing ‘-‘ sign on [s|d] block, what makes message a bit confused IMO. Maybe [-s|-d] would be more clear.

Both are wrong.

This is also broken in the man page, where the '|' is literal and
misformatted.  Normal markup would give '[-d | -s]' in the man page, and
this should be copied to the usage message.  Hard-coding the '|' using
's|d' gives the different syntax error of a hyphen before the 's'but no
hyphen before the 's' ('[-s|d]').

The hard-coding has 2 other bugs:
- missing spaces around '|'
- d and s are unsorted

d and s are unsorted consistently in about 8 instances in the patch.

The correct order for sorting -f and [-d | -s] in the synopsis and
usage message is unclear.  It would be best to put -f after -d and keep
-s attached to -d, but with longer options list this takes too much
space by splitting up the single-letter options.

Bruce