[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 01/37] virutil: Move string related functions to virstring.c



On 04/29/2013 07:50 AM, Michal Privoznik wrote:
> The source code base needs to be adapted as well. Some files
> include virutil.h just for the string related functions (here,
> the include is substituted to match the new file), some include
> virutil.h without any need (here, the include is removed), and
> some require both.
> ---
>  cfg.mk                                    |   4 +-

>  src/util/virstoragefile.c                 |   2 +
>  src/util/virstring.c                      | 335 ++++++++++++++++++++++++++++++
>  src/util/virstring.h                      |  50 +++++
>  src/util/virsysinfo.c                     |   2 +-
>  src/util/virtime.c                        |   1 -
>  src/util/virtpm.c                         |   2 +-
>  src/util/virtypedparam.c                  |   1 +
>  src/util/viruri.c                         |   2 +-
>  src/util/virusb.c                         |   1 +
>  src/util/virutil.c                        | 334 +----------------------------

Hmm, I guess git rename detection only kicks in for entire files, not
for big chunks of code motion between existing files.

>  242 files changed, 590 insertions(+), 557 deletions(-)

Growth is explainable by the number of new #include needed, I hope.

The fact that things still compile is good; I'm not reviewing too
closely, as the compiler should catch whether the motion was accurate.
I'm guessing you just removed #include "virutil.h" everywhere, then
compiled and fixed compiler errors for missing usage, until you had the
correct set of includes added back in?

> +++ b/src/libvirt_private.syms
> @@ -1745,10 +1745,26 @@ virStorageFileResize;
>  
>  
>  # util/virstring.h
> +virArgvToString;
> +virAsprintf;
> +virSkipSpaces;
> +virSkipSpacesAndBackslash;
> +virSkipSpacesBackwards;
> +virStrcpy;
>  virStringArrayHasString;
>  virStringFreeList;
>  virStringJoin;
>  virStringSplit;
> +virStrncpy;
> +virStrToDouble;
> +virStrToLong_i;
> +virStrToLong_l;
> +virStrToLong_ll;
> +virStrToLong_ui;
> +virStrToLong_ul;
> +virStrToLong_ull;
> +virTrimSpaces;
> +virVasprintf;
>  

Looks like a reasonable set to be moved.

> +++ b/src/util/virstring.c
> @@ -21,6 +21,10 @@
>  
>  #include <config.h>
>  
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "c-ctype.h"
>  #include "virstring.h"
>  #include "viralloc.h"
>  #include "virbuffer.h"
> @@ -180,3 +184,334 @@ virStringArrayHasString(char **strings, const char *needle)
>  
>      return false;
>  }
> +
> +/* Like strtol, but produce an "int" result, and check more carefully.
> +   Return 0 upon success;  return -1 to indicate failure.
> +   When END_PTR is NULL, the byte after the final valid digit must be NUL.
> +   Otherwise, it's like strtol and lets the caller check any suffix for
> +   validity.  This function is careful to return -1 when the string S
> +   represents a number that is not representable as an "int". */
> +int
> +virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
> +{
> +    long int val;
> +    char *p;
> +    int err;
> +
> +    errno = 0;
> +    val = strtol(s, &p, base); /* exempt from syntax-check */
> +    err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
> +    if (end_ptr)
> +        *end_ptr = p;
> +    if (err)
> +        return -1;
> +    *result = val;
> +    return 0;
> +}

I did a spot check that this function did straight code motion; I'm
assuming that for all the others, you did likewise.

> +++ b/src/util/virutil.c

> -/* Like strtol, but produce an "int" result, and check more carefully.
> -   Return 0 upon success;  return -1 to indicate failure.
> -   When END_PTR is NULL, the byte after the final valid digit must be NUL.
> -   Otherwise, it's like strtol and lets the caller check any suffix for
> -   validity.  This function is careful to return -1 when the string S
> -   represents a number that is not representable as an "int". */
> -int
> -virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
> -{
> -    long int val;
> -    char *p;
> -    int err;
> -
> -    errno = 0;
> -    val = strtol(s, &p, base); /* exempt from syntax-check */
> -    err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
> -    if (end_ptr)
> -        *end_ptr = p;
> -    if (err)
> -        return -1;
> -    *result = val;
> -    return 0;
> -}

ACK.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]