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

[Michael Tokarev]

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/ ?

Thanks,

/mjt