RE: [XEN PATCH for-4.14] tools/xen-ucode: fix error code propagation of microcode load operation

[Paul Durrant <xadimgnik@gmail.com>]

> -----Original Message-----
> From: Ian Jackson <ian.jackson@citrix.com>
> Sent: 12 June 2020 17:54
> To: Igor Druzhinin <igor.druzhinin@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Andrew Cooper <Andrew.Cooper3@citrix.com>; wl@xen.org; Paul
> Durrant <xadimgnik@gmail.com>
> Subject: Re: [XEN PATCH for-4.14] tools/xen-ucode: fix error code propagation of microcode load
> operation
> 
> Igor Druzhinin writes ("[PATCH] tools/xen-ucode: fix error code propagation of microcode load
> operation"):
> > Otherwise it's impossible to know the reason for a fault or blob rejection
> > inside the automation.
> ...
> >          fprintf(stderr, "Failed to update microcode. (err: %s)\n",
> >                  strerror(errno));
> 
> This part is fine.
> 
> > +        ret = errno;
> >      xc_interface_close(xch);
> ...
> >      }
> >      close(fd);
> >
> > -    return 0;
> > +    return ret;
> 
> Unfortunately I don't think this is right.  errno might not fit into a
> return value.

I don't understand this part. Why would errno not fit? 

>  Returning nonzero on microcode loading error would
> definitely be right, but ...
> 
> ... oh I have just read the rest of this file.
> 
> I think what is missing here is simply `return errno' (and the braces)
> There is no need to call xc_interface_close, or munmap, if we are
> about to exit.
> 
> I think fixing the lost error return is 4.14 material, so I have
> added that to the subject line.
> 
> Paul, would you Release-ack a patch that replaced every `return errno'
> with (say) exit(12) ?

Why 12?

>  Otherwise, fixing this program not to try to
> fit errno into an exit status is future work.  Also I notice that the
> program exits 0 if invoked wrongly.  Unhelpful!  I would want to fix
> that too.

Agreed that is unhelpful. I'm happy in principle to release-ack something replacing the returns with exits.

  Paul

> 
> Ian.