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 - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

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

-N doesn't need the -b bootnum.

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

Thanks, fixed.

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

usr.sbin/efibootmgr/efibootmgr.8
54

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
60

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

197

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.

202

getprogname() is the preferred interface

205

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.

289

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 edited the summary of this revision. (Show Details)

Address comments from @imp.

yuripv added inline comments.
usr.sbin/efibootmgr/efibootmgr.c
197

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

@imp anything else I need to change here?

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
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?

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?

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