[libvirt] [PATCH] Fix safezero()

[Daniel Veillard]

On Tue, Mar 02, 2010 at 04:11:24PM +0100, Jiri Denemark wrote:
> Various safezero() implementations used either -1, errno or -errno
> return values. This patch fixes them all to return -1 and set errno
> appropriately.
> 
> There was also a bug in size parameter passed to safewrite() which could
> result in an attempt to write gigabytes out of a megabyte buffer.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  src/storage/storage_backend.c |    4 ++--
>  src/util/util.c               |   16 +++++++++-------
>  2 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 3742493..849f01b 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -316,7 +316,7 @@ static int createRawFileOpHook(int fd, void *data) {
>                  if ((r = safezero(fd, 0, hdata->vol->allocation - remain,
>                                    bytes)) != 0) {
>                      ret = errno;
> -                    virReportSystemError(r, _("cannot fill file '%s'"),
> +                    virReportSystemError(errno, _("cannot fill file '%s'"),
>                                           hdata->vol->target.path);
>                      goto cleanup;
>                  }
> @@ -327,7 +327,7 @@ static int createRawFileOpHook(int fd, void *data) {
>  
>              if ((r = safezero(fd, 0, 0, remain)) != 0) {
>                  ret = errno;
> -                virReportSystemError(r, _("cannot fill file '%s'"),
> +                virReportSystemError(errno, _("cannot fill file '%s'"),
>                                       hdata->vol->target.path);
>                  goto cleanup;
>              }
> diff --git a/src/util/util.c b/src/util/util.c
> index cf7bba5..34c585d 100644
> --- a/src/util/util.c
> +++ b/src/util/util.c
> @@ -146,11 +146,11 @@ int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len)
>       */
>      r = ftruncate(fd, offset + len);
>      if (r < 0)
> -        return -errno;
> +        return -1;
>  
>      buf = mmap(NULL, len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, offset);
>      if (buf == MAP_FAILED)
> -        return -errno;
> +        return -1;
>  
>      memset(buf, 0, len);
>      munmap(buf, len);
> @@ -167,24 +167,26 @@ int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len)
>      unsigned long long remain, bytes;
>  
>      if (lseek(fd, offset, SEEK_SET) < 0)
> -        return errno;
> +        return -1;
>  
>      /* Split up the write in small chunks so as not to allocate lots of RAM */
>      remain = len;
>      bytes = 1024 * 1024;
>  
>      r = VIR_ALLOC_N(buf, bytes);
> -    if (r < 0)
> -        return -ENOMEM;
> +    if (r < 0) {
> +        errno = ENOMEM;
> +        return -1;
> +    }
>  
>      while (remain) {
>          if (bytes > remain)
>              bytes = remain;
>  
> -        r = safewrite(fd, buf, len);
> +        r = safewrite(fd, buf, bytes);
>          if (r < 0) {
>              VIR_FREE(buf);
> -            return r;
> +            return -1;
>          }
>  
>          /* safewrite() guarantees all data will be written */

  ACK, this is bug fix from my viewpoint, so I'm fine pushing this now,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/