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

Re: [libvirt] [PATCHv2 1/2] virBitmapParse: Fix behavior in case of error



On Tue, Aug 20, 2013 at 11:33:43AM +0200, Peter Krempa wrote:
> Re-arrange the code so that the returned bitmap is always initialized to
> NULL even on early failures and return an error message as some callers
> are already expecting it.
> ---
> 
> Notes:
>     Version 2:
>     Was already ACKed in v1, but:
>     * fixed bracing of arguments of the _() macro
>     * added src/uti/virbitmap.c to po/POTFILES.in
>     (noticed by doing a full build ... )
> 
>  po/POTFILES.in       |  1 +
>  src/util/virbitmap.c | 18 +++++++++---------
>  2 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 36d027a..9a83069 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -147,6 +147,7 @@ src/util/viralloc.c
>  src/util/viraudit.c
>  src/util/virauth.c
>  src/util/virauthconfig.c
> +src/util/virbitmap.c
>  src/util/vircgroup.c
>  src/util/virclosecallbacks.c
>  src/util/vircommand.c
> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> index 47c678e..289a7b9 100644
> --- a/src/util/virbitmap.c
> +++ b/src/util/virbitmap.c
> @@ -298,23 +298,21 @@ virBitmapParse(const char *str,
>                 size_t bitmapSize)
>  {
>      bool neg = false;
> -    const char *cur;
> +    const char *cur = str;
>      char *tmp;
>      size_t i;
>      int start, last;
> 
> -    if (!str)
> +    if (!(*bitmap = virBitmapNew(bitmapSize)))
>          return -1;
> 
> -    cur = str;
> -    virSkipSpaces(&cur);
> +    if (!str)
> +        goto error;
> 
> -    if (*cur == 0)
> -        return -1;
> +    virSkipSpaces(&cur);
> 
> -    *bitmap = virBitmapNew(bitmapSize);
> -    if (!*bitmap)
> -        return -1;
> +    if (*cur == '\0')
> +        goto error;
> 
>      while (*cur != 0 && *cur != terminator) {
>          /*
> @@ -384,6 +382,8 @@ virBitmapParse(const char *str,
>      return virBitmapCountBits(*bitmap);
> 
>  error:
> +    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                   _("Failed to parse bitmap '%s'"), str);

If you're going to add this, then you must fix all the callers
which currently report their own error.


# git grep --after 4 virBitmapParse src/

Shows a great many needing fixing.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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