Re: [PATCH 1/4] virStorageVol: avoid PATH_MAX-sized array

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

 



2011/6/22 Eric Blake <eblake@xxxxxxxxxx>:
> POSIX allows implementations where PATH_MAX is undefined, leading
> to compilation error.  Not to mention that even if it is defined,
> it is often wasteful in relation to the amount of data being stored.
>
> All clients of vol->key were audited, and found not to care about
> whether key is static or dynamic, except for these offenders:
>
> * src/datatypes.h (struct _virStorageVol): Manage key dynamically.
> * src/datatypes.c (virReleaseStorageVol): Free key.
> (virGetStorageVol): Copy key.
> ---
>
> I'm also adding a 'make syntax-check' rule in gnulib to catch this
> from being a problem in the future:
> http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/27259
>
>  src/datatypes.c |    8 +++++---
>  src/datatypes.h |    5 ++---
>  2 files changed, 7 insertions(+), 6 deletions(-)

> @@ -254,8 +254,7 @@ struct _virStorageVol {
>     virConnectPtr conn;                  /* pointer back to the connection */
>     char *pool;                          /* Pool name of owner */
>     char *name;                          /* the storage vol external name */
> -    /* XXX currently abusing path for this. Ought not to be so evil */
> -    char key[PATH_MAX];                  /* unique key for storage vol */
> +    char *key;                           /* unique key for storage vol */
>  };

I think that the comment was referring to the some storage backends
using the path of a file as the key instead of some other form of key.
So the comment may still be valid (but only for specific backend) and
was not related to PATH_MAX here. As it's definitely wrong to have it
in this general place I think it's okay to remove it.

ACK.

-- 
Matthias Bolte
http://photron.blogspot.com

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]