mixer: Improve error messages

[fernape]

Summary

Use better diagnostic messages when unit numbers are wrong.

This is an example of the current and proposed behavior:

$ mixer -d pcmAAA
mixer: strtol(AAA): Invalid argument

$mixer -dpcmAAAA
mixer: Can not convert 'AAAA' to a number: Invalid argument

$ mixer -d999999999999999999999999999999999
mixer: strtol(999999999999999999999999999999999): Result too large

$mixer -d99999999999999999999999
mixer: 99999999999999999999999: Result too large

$ mixer -d-2
mixer: cannot set default unit to -2: Invalid argument
pcm3:mixer: <Realtek ALC887 (Front Analog)> on hdaa1 (play/rec) (default)
    vol       = 0.78:0.78     pbk
    pcm       = 0.85:0.85     pbk
    speaker   = 0.00:0.00     rec
    rec       = 0.35:0.35     pbk
    igain     = 0.00:0.00     pbk
    ogain     = 1.00:1.00     pbk
    monitor   = 0.67:0.67     rec src

$mixer -d-2
mixer: Unit number must be a positive number

$ mixer -d99
mixer: cannot set default unit to 99: Invalid argument
pcm3:mixer: <Realtek ALC887 (Front Analog)> on hdaa1 (play/rec) (default)
    vol       = 0.78:0.78     pbk
    pcm       = 0.85:0.85     pbk
    speaker   = 0.00:0.00     rec
    rec       = 0.35:0.35     pbk
    igain     = 0.00:0.00     pbk
    ogain     = 1.00:1.00     pbk
    monitor   = 0.67:0.67     rec src

$mixer -d99
mixer: No such mixer unit: 99

Test Plan

Apply attached patch and run the commands above.

[fernape]

Friendly ping 🔔 !!

[arrowd]

usr.sbin/mixer/mixer.c

76

That doesn't look like a clear error message?

[fernape]

usr.sbin/mixer/mixer.c

76

Now it does because it removes the strtol text. Or do you think something else? The new behavior is:

mixer -d9999999999999999999999999999999999999999999999999
mixer: 9999999999999999999999999999999999999999999999999: Result too large

[arrowd]

usr.sbin/mixer/mixer.c

76

Right, I forgot that err prints a textual errno representation too.

[christos]

To be fair, I don't think these changes are meaningful. mixer(8) will fail for all the cases you are trying to catch anyway, and I think the messages are clear enough already.

Negative dunit:

$ mixer -d -1
mixer: cannot set default unit to -1: Invalid argument

dunit larger than the total number of mixers:

$ mixer -d10
mixer: cannot set default unit to 10: Invalid argument

Too large number:

$ mixer -d9999999999999999999999
mixer: strtol(9999999999999999999999): Result too large

Invalid input:

$ mixer -dfoobar
mixer: strtol(foobar): Invalid argument

The current scheme is uniform, and if a user wants to use the -d option in the first place, they should know that the argument is supposed to be a unit number (or pcm<unit_number>), so the current messages point to what the error is pretty well IMHO.

[fernape]

In D56845#1348954, @christos wrote:

To be fair, I don't think these changes are meaningful. mixer(8) will fail for all the cases you are trying to catch anyway, and I think the messages are clear enough already.

Negative dunit:

$ mixer -d -1
mixer: cannot set default unit to -1: Invalid argument

dunit larger than the total number of mixers:

$ mixer -d10
mixer: cannot set default unit to 10: Invalid argument

Too large number:

$ mixer -d9999999999999999999999
mixer: strtol(9999999999999999999999): Result too large

Invalid input:

$ mixer -dfoobar
mixer: strtol(foobar): Invalid argument

The current scheme is uniform, and if a user wants to use the -d option in the first place, they should know that the argument is supposed to be a unit number (or pcm<unit_number>), so the current messages point to what the error is pretty well IMHO.

I would argue that the user might not know it. "Invalid argument" is barely descriptive IMHO. If mixer knows the problem is that the supplied device number is negative, why not tell the user the exact problem instead of trusting the user "should know" something?

The last two examples (strtol(...) ... Invalid argument) don't make sense from a user point of view. What is strtol()? Is a *user*, not a C developer, suppose to know what that is? mixer *is assuming* the user knows what this C library function does which isn't right I think.
I think explicit messages are better from the user perspective.