On 14 February 2017 at 10:25, Markus Armbruster <arm...@redhat.com> wrote:
> Reorder check_strtox_error() to make it obvious that we always store
> through a non-null @endptr.
>
> Transform
>
> if (some error) {
> error case ...
> err = value for error case;
> } else {
> normal case ...
> err = value for normal case;
> }
> return err;
>
> to
>
> if (some error) {
> error case ...
> return value for error case;
> }
> normal case ...
> return value for normal case;
>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> util/cutils.c | 89
> ++++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 45 insertions(+), 44 deletions(-)
>
> diff --git a/util/cutils.c b/util/cutils.c
> index 7d165bc..7442d46 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -265,15 +265,20 @@ int64_t qemu_strtosz(const char *nptr, char **end)
> static int check_strtox_error(const char *nptr, char *ep,
> const char **endptr, int eno)
> {
> - if (eno == 0 && ep == nptr) {
> - eno = EINVAL;
> - }
> - if (!endptr && *ep) {
> - return -EINVAL;
> - }
> if (endptr) {
> *endptr = ep;
> }
> +
> + /* Turn "no conversion" into an error */
> + if (eno == 0 && ep == nptr) {
> + return -EINVAL;
> + }
> +
> + /* Fail when we're expected to consume the string, but didn't */
> + if (!endptr && *ep) {
> + return -EINVAL;
> + }
> +
> return -eno;
> }
Doesn't this change the semantics? Previously, for the
case of (eno == 0 && ep == nptr) we would both set
*endptr to ep and return -EINVAL. Now we only return -EINVAL
and leave *endptr alone. The comment describing the
qemu_strtol() API is a bit vague about what exactly we keep
from the strtol() semantics for "no convertable characters"
but I would assume that retaining "*endptr is written with
the original value of nptr" is intentional.
thanks
-- PMM
