Re: [Qemu-devel] [RFC PATCH 05/11] qemu_fclose: return last_error if set

[Paolo Bonzini <pbonzini@redhat.com>]

On 11/02/2011 12:56 PM, Eduardo Habkost wrote:
> No, if it's positive it won't be set on last_error, so we have to save
> it somewhere other than last_error (that's what the qemu_close() return
> value is used for).

Ok, I was confused by your patch 6, which basically removes the only 
case when qemu_fclose was returning a positive, nonzero value. :)  I 
guess the problem is there?

> Without a separate function and qemu_file_set_if_error(), the function
> will look like:
>
> int qemu_fclose(QEMUFile *f)
> {
>      int ret = 0;
>      qemu_fflush();
>      if (f->close) {
>          ret = f->close(f->opaque);
>      }
>      if (f->last_error) {
            ^^^^^^^^^^^^^

"if (ret >= 0 && f->last_error)" perhaps?

>          ret = f->last_error;
>      }
>      g_free(f);
>      return ret;
> }
>
>
> Now that I am looking at the resulting code, it doesn't look too bad. I
> guess I was simply too eager to encapsulate every bit of logic (in this
> case the "if (f->close) ..." part) into separate functions. I find the
> two-function version slightly easier to analyze, though.

Yes, it's the same for me too now that I actually understand what's 
going on.

Paolo