All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: "Carlos L. Torres" <cltorrespr@gmail.com>, qemu-devel@nongnu.org
Cc: stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH 1/4] cutils: Add qemu_strtol() wrapper
Date: Wed, 8 Jul 2015 14:59:55 +0200	[thread overview]
Message-ID: <559D1ECB.6000204@redhat.com> (raw)
In-Reply-To: <79656e01ce28a5fec2bf6388d3d92f881e6a6e34.1436317535.git.carlos.torres@rackspace.com>



On 08/07/2015 03:09, Carlos L. Torres wrote:
> From: "Carlos L. Torres" <carlos.torres@rackspace.com>
> 
> Add wrapper for strtol() function. Include unit tests.
> 
> Signed-off-by: Carlos L. Torres <carlos.torres@rackspace.com>

Very thorough work, just a couple comments below.

> ---
>  include/qemu-common.h |   2 +
>  tests/test-cutils.c   | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  util/cutils.c         |  58 ++++++++++
>  3 files changed, 352 insertions(+)
> 
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 237d654..a0ce7d8 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -161,6 +161,8 @@ int qemu_fls(int i);
>  int qemu_fdatasync(int fd);
>  int fcntl_setfl(int fd, int flag);
>  int qemu_parse_fd(const char *param);
> +int qemu_strtol(const char *nptr, const char **endptr, int base,
> +                long *result);
>  
>  int parse_uint(const char *s, unsigned long long *value, char **endptr,
>                 int base);
> diff --git a/tests/test-cutils.c b/tests/test-cutils.c
> index 2a4556d..0e8fdbf 100644
> --- a/tests/test-cutils.c
> +++ b/tests/test-cutils.c
> @@ -226,6 +226,269 @@ static void test_parse_uint_full_correct(void)
>      g_assert_cmpint(i, ==, 123);
>  }
>  
> +static void test_qemu_strtol_correct(void)
> +{
> +    const char *str = "12345 foo";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 12345);
> +    g_assert(endptr == str + 5);
> +}
> +
> +static void test_qemu_strtol_null(void)
> +{
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(NULL, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == NULL);
> +}
> +
> +static void test_qemu_strtol_empty(void)
> +{
> +    const char *str = "";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0);

Please assert that endptr == str.

> +}
> +
> +static void test_qemu_strtol_whitespace(void)
> +{
> +    const char *str = "  \t  ";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtol_invalid(void)
> +{
> +    const char *str = "   xxxx  \t abc";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0);
> +    g_assert(endptr == str);
> +}
> +
> +static void test_qemu_strtol_trailing(void)
> +{
> +    const char *str = "123xxx";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +    g_assert(endptr == str + 3);
> +}
> +
> +static void test_qemu_strtol_octal(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 8, &res);

Add a testcase for str = "0123" and base = 0?

> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_decimal(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 10, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_hex(void)
> +{
> +    const char *str = "0123";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 16, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0x123);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_max(void)
> +{
> +    const char *str = g_strdup_printf("%ld", LONG_MAX);
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, LONG_MAX);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_overflow(void)
> +{
> +    const char *str = "99999999999999999999999999999999999999999999";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -ERANGE);
> +    g_assert_cmpint(res, ==, LONG_MAX);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_underflow(void)
> +{
> +    const char *str = "-99999999999999999999999999999999999999999999";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err  = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -ERANGE);
> +    g_assert_cmpint(res, ==, LONG_MIN);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_negative(void)
> +{
> +    const char *str = "  \t -321";
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, -321);
> +    g_assert(endptr == str + strlen(str));
> +}
> +
> +static void test_qemu_strtol_full_correct(void)
> +{
> +    const char *str = "123";
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 123);
> +}
> +
> +static void test_qemu_strtol_full_null(void)
> +{
> +    char f = 'X';
> +    const char *endptr = &f;
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(NULL, &endptr, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +    g_assert(endptr == NULL);
> +}
> +
> +static void test_qemu_strtol_full_empty(void)
> +{
> +    const char *str = "";
> +    long res = 999L;
> +    int err;
> +
> +    err =  qemu_strtol(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, 0);
> +}
> +
> +static void test_qemu_strtol_full_negative(void)
> +{
> +    const char *str = " \t -321";
> +    long res = 999;
> +    int err;
> +
> +    err = qemu_strtol(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, -321);
> +}
> +
> +static void test_qemu_strtol_full_trailing(void)
> +{
> +    const char *str = "123xxx";
> +    long res;
> +    int err;
> +
> +    err = qemu_strtol(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, -EINVAL);
> +}
> +
> +static void test_qemu_strtol_full_max(void)
> +{
> +    const char *str = g_strdup_printf("%ld", LONG_MAX);
> +    long res;
> +    int err;
> +
> +    err = qemu_strtol(str, NULL, 0, &res);
> +
> +    g_assert_cmpint(err, ==, 0);
> +    g_assert_cmpint(res, ==, LONG_MAX);
> +}
>  int main(int argc, char **argv)
>  {
>      g_test_init(&argc, &argv, NULL);
> @@ -247,5 +510,34 @@ int main(int argc, char **argv)
>      g_test_add_func("/cutils/parse_uint_full/correct",
>                      test_parse_uint_full_correct);
>  
> +    /* qemu_strtol() tests */
> +    g_test_add_func("/cutils/qemu_strtol/correct", test_qemu_strtol_correct);
> +    g_test_add_func("/cutils/qemu_strtol/null", test_qemu_strtol_null);
> +    g_test_add_func("/cutils/qemu_strtol/empty", test_qemu_strtol_empty);
> +    g_test_add_func("/cutils/qemu_strtol/whitespace",
> +                    test_qemu_strtol_whitespace);
> +    g_test_add_func("/cutils/qemu_strtol/invalid", test_qemu_strtol_invalid);
> +    g_test_add_func("/cutils/qemu_strtol/trailing", test_qemu_strtol_trailing);
> +    g_test_add_func("/cutils/qemu_strtol/octal", test_qemu_strtol_octal);
> +    g_test_add_func("/cutils/qemu_strtol/decimal", test_qemu_strtol_decimal);
> +    g_test_add_func("/cutils/qemu_strtol/hex", test_qemu_strtol_hex);
> +    g_test_add_func("/cutils/qemu_strtol/max", test_qemu_strtol_max);
> +    g_test_add_func("/cutils/qemu_strtol/overflow", test_qemu_strtol_overflow);
> +    g_test_add_func("/cutils/qemu_strtol/underflow",
> +                    test_qemu_strtol_underflow);
> +    g_test_add_func("/cutils/qemu_strtol/negative", test_qemu_strtol_negative);
> +    g_test_add_func("/cutils/qemu_strtol_full/correct",
> +                    test_qemu_strtol_full_correct);
> +    g_test_add_func("/cutils/qemu_strtol_full/null",
> +                    test_qemu_strtol_full_null);
> +    g_test_add_func("/cutils/qemu_strtol_full/empty",
> +                    test_qemu_strtol_full_empty);
> +    g_test_add_func("/cutils/qemu_strtol_full/negative",
> +                    test_qemu_strtol_full_negative);
> +    g_test_add_func("/cutils/qemu_strtol_full/trailing",
> +                    test_qemu_strtol_full_trailing);
> +    g_test_add_func("/cutils/qemu_strtol_full/max",
> +                    test_qemu_strtol_full_max);
> +
>      return g_test_run();
>  }
> diff --git a/util/cutils.c b/util/cutils.c
> index 5d1c9eb..bfaccd3 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -359,6 +359,64 @@ int64_t strtosz(const char *nptr, char **end)
>  }
>  
>  /**
> + * Helper function for qemu_strto*l() functions.
> + */
> +static int check_strtox_error(const char **next, char *endptr,
> +                              int err)
> +{
> +    if (!next && *endptr) {
> +        return -EINVAL;
> +    }
> +    if (next) {
> +        *next = endptr;
> +    }
> +    return -err;
> +}
> +
> +/**
> + * QEMU wrappers for strtol(), strtoll(), strtoul(), strotull() C functions.
> + *
> + * Convert ASCII string @nptr to a long integer value
> + * from the given @base. Parameters @nptr, @endptr, @base
> + * follows same semantics as strtol() C function.
> + *
> + * Different from strtol() function, if @endptr is not NULL, this

"Unlike the strtol() function," ...

> + * function will return -EINVAL whenever it cannot fully convert
> + * the string in @nptr with given @base to a long. This function returns
> + * the result of the conversion only through the @result parameter.
> + *
> + * If NULL is passed in @endptr, then the whole string in @ntpr
> + * is a number otherwise it returns -EINVAL.
> + *
> + * RETURN VALUE
> + * Different from strtol() function, this wrapper returns either

Same here.

> + * -EINVAL or the errno set by strtol() function (e.g -ERANGE).
> + * If the conversion overflows, -ERANGE is returned, and @result
> + * is set to the max value of the desired type
> + * (e.g. LONG_MAX, LLONG_MAX, ULONG_MAX, ULLONG_MAX). If the case
> + * of underflow, -ERANGE is returned, and @result is set to the min
> + * value of the desired type. For strtol(), strtoll(), @result is set to
> + * LONG_MIN, LLONG_MIN, respectively, and for strtoul(), strtoull() it
> + * is set to 0.
> + */
> +int qemu_strtol(const char *nptr, const char **endptr, int base,
> +                long *result)
> +{
> +    char *p;
> +    int err = 0;
> +    if (!nptr) {
> +        if (endptr) {
> +            *endptr = nptr;
> +        }
> +        err = -EINVAL;
> +    } else {
> +        errno = 0;
> +        *result = strtol(nptr, &p, base);
> +        err = check_strtox_error(endptr, p, errno);
> +    }
> +    return err;
> +}
> +/**
>   * parse_uint:
>   *
>   * @s: String to parse
> 

  reply	other threads:[~2015-07-08 13:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-08  1:09 [Qemu-devel] [PATCH 0/4] cutils: Add qemu_strto*l() wrappers Carlos L. Torres
2015-07-08  1:09 ` [Qemu-devel] [PATCH 1/4] cutils: Add qemu_strtol() wrapper Carlos L. Torres
2015-07-08 12:59   ` Paolo Bonzini [this message]
2015-07-08  1:09 ` [Qemu-devel] [PATCH 2/4] cutils: Add qemu_strtoul() wrapper Carlos L. Torres
2015-07-08 13:01   ` Paolo Bonzini
2015-07-08  1:09 ` [Qemu-devel] [PATCH 3/4] cutils: Add qemu_strtoll() wrapper Carlos L. Torres
2015-07-08  1:09 ` [Qemu-devel] [PATCH 4/4] cutils: Add qemu_strtoull() wrapper Carlos L. Torres
2015-07-08 13:02 ` [Qemu-devel] [PATCH 0/4] cutils: Add qemu_strto*l() wrappers Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=559D1ECB.6000204@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=cltorrespr@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.