[Top][All Lists]

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

Re: [Qemu-trivial] [PATCH] arch_init: Simplify code for load_xbzrle()

From: Michael Tokarev
Subject: Re: [Qemu-trivial] [PATCH] arch_init: Simplify code for load_xbzrle()
Date: Sat, 17 May 2014 11:54:39 +0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0

10.05.2014 16:51, Chen Gang wrote:
> For xbzrle_decode_buffer(), when decoding contents will exceed writing
> buffer, it will return -1, so need not check the return value whether
> large than writing buffer.
> And when failure occurs within load_xbzrle(), it always return -1
> without any resources which need release.
> So can remove the related checking statements, and also can remove 'rc'
> and 'ret' local variables,

Just one comment below.

> @@ -933,18 +932,13 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, 
> void *host)
>      qemu_get_buffer(f, xbzrle_decoded_buf, xh_len);
>      /* decode RLE */
> -    ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
> -                               TARGET_PAGE_SIZE);
> -    if (ret == -1) {
> +    if (xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host,
> +                             TARGET_PAGE_SIZE) == -1) {

Can we compare like '< 0' here, not like '== -1' ?
Are there any other possible return values from xbzrle_decode_buffer()
which are less than zero but non-error?

To me, anything less than zero is always error (unless it is one of the
possible non-error values, like offset for example which can be negative).

Especially having in mind that in the future, some function may extend
its error return to include the actual error code (like -errno), in which
case code which compares with -1 will not work anymore.

Is it okay to me to apply this with s/== -1/< 0/ ?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]