Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 9 Nov 2016 12:42:18 +0800
From:      Marcelo Araujo <araujobsdport@gmail.com>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        Renato Botelho <garga@freebsd.org>,  "src-committers@freebsd.org" <src-committers@freebsd.org>,  "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>,  "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r308443 - head/bin/hostname
Message-ID:  <CAOfEmZgjxkG%2B15=6HnysABW74Weg4oAwTHQFEOftrBO1UcL=ng@mail.gmail.com>
In-Reply-To: <20161109115729.F924@besplex.bde.org>
References:  <201611081136.uA8BaXrs073937@repo.freebsd.org> <FCE8585E-BC70-4C80-B2D1-A11B461F8B91@FreeBSD.org> <20161109115729.F924@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Fixed, thanks!

2016-11-09 9:39 GMT+08:00 Bruce Evans <brde@optusnet.com.au>:

> On Tue, 8 Nov 2016, Renato Botelho wrote:
>
> On 8 Nov 2016, at 09:36, Marcelo Araujo <araujo@FreeBSD.org> wrote:
>>>
>>> Log:
>>>  Add -d flag that prints domain only.
>>>
>>>  PR:            212875
>>>  Submitted by:  Ben RUBSON <ben.rubson@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
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- 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
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- 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 =3D 0;
>>> -       while ((ch =3D getopt(argc, argv, "fs")) !=3D -1)
>>> +       dflag =3D 0;
>>> +       while ((ch =3D getopt(argc, argv, "fsd")) !=3D -1)
>>>                 switch (ch) {
>>>                 case 'f':
>>>                         /*
>>> @@ -70,6 +71,9 @@ main(int argc, char *argv[])
>>>                 case 's':
>>>                         sflag =3D 1;
>>>                         break;
>>> +               case 'd':
>>> +                       dflag =3D 1;
>>> +                       break;
>>>                 case '?':
>>>                 default:
>>>                         usage();
>>> @@ -77,7 +81,7 @@ main(int argc, char *argv[])
>>>         argc -=3D optind;
>>>         argv +=3D optind;
>>>
>>> -       if (argc > 1)
>>> +       if (argc > 1 || (sflag && dflag))
>>>                 usage();
>>>
>>>         if (*argv) {
>>> @@ -90,6 +94,10 @@ main(int argc, char *argv[])
>>>                         p =3D strchr(hostname, '.');
>>>                         if (p !=3D NULL)
>>>                                 *p =3D '\0';
>>> +               } else if (dflag) {
>>> +                       p =3D strchr(hostname, '.');
>>> +                       if (p !=3D 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=E2=80=99s missing =E2=80=98-=E2=80=98 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




--=20

--=20
Marcelo Araujo            (__)araujo@FreeBSD.org
\\\'',)http://www.FreeBSD.org <http://www.freebsd.org/>;   \/  \ ^
Power To Server.         .\. /_)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOfEmZgjxkG%2B15=6HnysABW74Weg4oAwTHQFEOftrBO1UcL=ng>