Re: [PATCH] conf: use proper maximum for parsing memory values

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

 



On 10/30/2014 09:49 AM, Martin Kletzander wrote:
> The value is stored in unsigned long long, so ULLONG_MAX is the proper
> upper limit to use.

No, it's not.

> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
> Even though this is a build-breaker (for 32bit systems
> memtune-unlimited fails to parse), I'm not pushing it as one because
> it feels odd that such change wouldn't break anything else.

What's the compile failure?  This patch is intentionally trying to fit
the largest value that will fit in an unsigned long when scaled by
kilobytes, while still parsing by bytes.  Thus, on 64-bit machines, you
can parse 0xffffffffffffffff bytes, then scale down by 1024 and store in
unsigned long; on 32-bit machines, your maximum has to be limited to
0xffffffff*1024 bytes before scaling back down.

> 
>  src/conf/domain_conf.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a351382..beb3d26 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -6388,16 +6388,10 @@ virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt,
>                       unsigned long long *mem, bool required)
>  {
>      int ret = -1;
> -    unsigned long long bytes, max;
> -
> -    /* On 32-bit machines, our bound is 0xffffffff * KiB. On 64-bit
> -     * machines, our bound is off_t (2^63).  */
> -    if (sizeof(unsigned long) < sizeof(long long))
> -        max = 1024ull * ULONG_MAX;
> -    else
> -        max = LLONG_MAX;
> +    unsigned long long bytes;
> 
> -    ret = virDomainParseScaledValue(xpath, ctxt, &bytes, 1024, max, required);
> +    ret = virDomainParseScaledValue(xpath, ctxt, &bytes, 1024,
> +                                    ULLONG_MAX, required);

NACK.  I need to see the compiler failure to see the proper fix, but
this fix is not going to work.


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

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]