Re: [PATCH] KVM: selftests: delete some dead code

[Peter Xu <peterx@redhat.com>]

On Fri, Jun 05, 2020 at 03:26:39PM +0200, Paolo Bonzini wrote:
> On 05/06/20 14:48, Peter Xu wrote:
> >>> The bug is that strtoul is "impossible" to use correctly.
> > Could I ask why?
> 
> To see see how annoying the situation is, check out utils/cutils.c in
> QEMU; basically, it is very hard to do error handling.  From the man page:
> 
>        Since  strtoul() can legitimately return 0 or ULONG_MAX
>        (ULLONG_MAX for strtoull()) on both success and failure, the
>        calling program should set errno to 0 before the call, and then
>        determine if an error occurred by checking whether errno has
>        a nonzero value after the call.
> 
> and of course no one wants to write code for that every time they have
> to parse a number.
> 
> In addition, if the string is empty it returns 0, and of endptr is NULL
> it will accept something like "123abc" and return 123.
> 
> So it is not literally impossible, but it is a poorly-designed interface
> which is a major source of bugs.  On Rusty's API design levels[1][2], I
> would put it at 3 if I'm feeling generous ("Read the documentation and
> you'll get it right"), and at -4 to -7 ("The obvious use is wrong") if
> it's been a bad day.
> 
> Therefore it's quite common to have a wrapper like
> 
>     int my_strtoul(char *p, char **endptr, unsigned long *result);
> 
> The wrapper will:
> 
> - check that the string is not empty
> 
> - always return 0 or -1 because of the by-reference output argument "result"
> 
> - take care of checking that the entire input string was parsed, for
> example by rejecting partial parsing of the string if endptr == NULL.
> 
> This version gets a solid 7 ("The obvious use is probably the correct
> one"); possibly even 8 ("The compiler will warn if you get it wrong")
> because the output argument gives you better protection against overflow.
> 
> Regarding overflow, there is a strtol but not a strtoi, so you need to
> have a temporary long and do range checking manually.  Again, you will
> most likely make mistakes if you use strtol, while my_strtol will merely
> make it annoying but it should be obvious that you're getting it wrong.
> 
> Paolo
> 
> [1] https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html
> [2] https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html

Fair enough, and a good reading material. :)

Thanks!

-- 
Peter Xu