On 02/20/2014 01:23 AM, Eric Sandeen wrote:
On 2/19/14, 10:31 AM, Goffredo Baroncelli wrote:
...
The error message says that the value is out of range. But doesn't tell which
is the parameter involved.
If you have a command which has several arguments, and the user pass a string
instead of a number, the error returned doesn't tell which argument is wrong.
This is the reason of my complaint.
At least add a fourth parameter which contains the name of the parameter
parsed in order to improve the error message.
I.E.
subvol_id = btrfs_strtoull(argv[i], 10, "subvolume ID");
If the user pass a path instead of a number the error message would be
ERROR: xxxx is not a valid unsigned long long integer for the parameter 'subvolume ID'.
Or something like that.
I'm not so sure that this is needed. Adding it makes the code a little more complicated,
and requires each caller to send in a string which may get out of sync with the actual
argument name (or the manpage, or the help/usage text).
The user has *just* typed it in, so it won't be too hard to see which one
was wrong even without naming the parameter, i.e. -
# btrfs foo-command 12345 0 123notanumber
Error: 123notanumber is not a valid numeric value
or
# btrfs baz-command 0xFFFFFFFFFFFFFFFF 12345 0
Error: numeric value 0xFFFFFFFFFFFFFFFF is too large.
would probably suffice, without bothering to name the parameter.
Sounds more reasonable.:-)
I'd also suggest not referring to "unsigned long long" or "out of range" which
might not mean much to people who aren't programmers; "not a valid value"
or "too large" might be clearer.
Will update it.
Also, what if someone enters a negative number? Right now it happily
accepts it and converts it to an unsigned value:
# ./test-64bit -123
Parsed as 18446744073709551493
So Let's add a '-' check before calling strtoull. something like:
if (str[0] == '-')
fprintf(stderr, "ERROR: this may be a negative integer: %s\n", str);
Thanks,
Wang
but an inadvertent negative sign might lead to highly unexpected results.
Just my $0.02.
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
