Page MenuHomeFreeBSD

efibootmgr: make generic usage message more readable
AbandonedPublic

Authored by yuripv on Jan 1 2019, 6:32 AM.

Details

Reviewers
imp
bcran
Group Reviewers
manpages
Summary
  • make the generic usage message more readable by splitting one long line into separate action options which can't (shouldn't) be used together
  • use {} instead of [] for alternative options
  • update man page accordingly.

New output:

# efibootmgr -h; echo $?
Usage:
        efibootmgr [-v]
        efibootmgr -c -l loader [-a] [-b bootnum] [-k kernel] [-L label]
        efibootmgr -B -b bootnum
        efibootmgr {-a|-A} -b bootnum
        efibootmgr {-n|-N} -b bootnum
        efibootmgr -o bootvarnum1,bootvarnum2,...
        efibootmgr -t seconds
        efibootmgr -T
0
# efibootmgr -w; echo $?
efibootmgr: invalid option -- w
Usage:
        efibootmgr [-v]
        efibootmgr -c -l loader [-a] [-b bootnum] [-k kernel] [-L label]
        efibootmgr -B -b bootnum
        efibootmgr {-a|-A} -b bootnum
        efibootmgr {-n|-N} -b bootnum
        efibootmgr -o bootvarnum1,bootvarnum2,...
        efibootmgr -t seconds
        efibootmgr -T
1
# efibootmgr -c; echo $?
Usage:
        efibootmgr -c -l loader [-a] [-b bootnum] [-k kernel] [-L label]
1

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

yuripv created this revision.Jan 1 2019, 6:32 AM
yuripv edited reviewers, added: bcran; removed: db.Jan 2 2019, 1:35 PM
bcran added a comment.Jan 2 2019, 2:28 PM

-N (delete BootNext) doesn’t require -b since it doesn’t operate on a bootnum.

yuripv updated this revision to Diff 52479.Jan 2 2019, 2:54 PM

-N doesn't need the -b bootnum.

yuripv added a comment.Jan 2 2019, 2:54 PM

-N (delete BootNext) doesn’t require -b since it doesn’t operate on a bootnum.

Thanks, fixed.

bcran added a comment.Jan 2 2019, 4:08 PM

Thanks! I like these changes - the single line was confusing.

usr.sbin/efibootmgr/efibootmgr.8
55

I'd prefer bootnum1, bootnum2 - to match the use of 'bootnum' in the rest of the page.

imp requested changes to this revision.Jan 2 2019, 4:19 PM
usr.sbin/efibootmgr/efibootmgr.c
61

Don't do this. Use getprogname() instead.

198

We should always send usage message to stderr.
What's the advantage of doing it differently for help text?
We don't do that anywhere else in the tree I'm aware of.

203

getprogname() is the preferred interface

206

We should always exit with status 1 here. We can eliminate the help bool which would also simplify things.
We don't make this distinction elsewhere in the tree that I can see.

290

Compilers are getting pickier and pickier. This is no valid C++, for example, and the trend is to having enums move away from being just an int into something more. IF you going to go for an enum, go all in and define one here too.

378

This is good, but should be a separate commit.

This revision now requires changes to proceed.Jan 2 2019, 4:19 PM
yuripv updated this revision to Diff 52480.Jan 2 2019, 4:45 PM
yuripv edited the summary of this revision. (Show Details)

Address comments from @imp.

yuripv marked 7 inline comments as done.Jan 2 2019, 4:46 PM
yuripv added inline comments.
usr.sbin/efibootmgr/efibootmgr.c
198

*Some* utilities do (e.g. camcontrol help and zfs/zpool from contrib), but it doesn't really matter much, changed.

yuripv marked an inline comment as done.Jan 2 2019, 4:47 PM
bcran accepted this revision.Jan 2 2019, 4:51 PM

@imp anything else I need to change here?

imp accepted this revision.Jan 12 2019, 5:49 PM

This is fine, but there's a chance we'll rework the arg parsing in this program in the near future... Better to get this in now though.

This revision is now accepted and ready to land.Jan 12 2019, 5:49 PM
yuripv added a comment.Feb 6 2019, 4:04 AM
In D18705#401939, @imp wrote:

This is fine, but there's a chance we'll rework the arg parsing in this program in the near future... Better to get this in now though.

I'm sorry for the delay on this. Warner, are you still ok with this going in?

imp added a comment.Feb 18 2019, 8:20 PM

Go ahead and commit. This won't collide with anything I have out of the tree at the moment, and it's better to have better messages in the mean time.

@yuripv Are you still planning to commit this?

yuripv abandoned this revision.Thu, Sep 19, 8:22 AM

As noted by @imp, there are better ways to do this.