linuxkpi: Don't clobber result on failure

[des]

Summary

In kstrto*(), don't assign to *res until we know the conversion is
successful, and address issues that may result in warnings if code
that uses <linux/kstrtox.h> is compiled at high warning levels.

MFC after: 1 week
Sponsored by: Klara, Inc.
Sponsored by: NetApp, Inc.

[des]

I've verified that graphics/drm-66-kmod builds cleanly with these changes.

[bz]

I think I would also always put an (at least) MPASS in there that res is not NULL.
Funny enough kstrtobool() has the check and fails gracefully if *res is NULL.

sys/compat/linuxkpi/common/include/linux/kstrtox.h

82

This may also take a leading + but no minus according to the documentation (unrelated to this fix). Just in case someone wants to do a follow-up.

[bz]

sys/compat/linuxkpi/common/include/linux/kstrtox.h

82

Forgive me. kstrtoul*l* does, not the one with just one 'l'. The double-ll is further down and would need the check before calling into the other one.

246

This one but not kstrtou64() may also take a leading '+'. (irrelevant to this review; sorry; see above)

[des]

sys/compat/linuxkpi/common/include/linux/kstrtox.h

82

I think perhaps someone should write a test suite for the entire family and run it on Linux, and then we can adjust our implementations to match.

[des]

In D54440#1244012, @bz wrote:

I think I would also always put an (at least) MPASS in there that res is not NULL.

I'm not sure there's much value in that. A null res is a guaranteed and trivially debugged panic anyway. An assertion makes no difference unless you put it at the top so it triggers even when the conversion would have failed.

Funny enough kstrtobool() has the check and fails gracefully if *res is NULL.

I'd argue that this is a mistake since it converts a guaranteed panic into something that's much harder to debug.