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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Ian Jackson <ian.jackson@xxxxxxxxxx>
> Sent: 12 June 2020 17:54
> To: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Andrew Cooper 
> <Andrew.Cooper3@xxxxxxxxxx>; wl@xxxxxxx; Paul
> Durrant <xadimgnik@xxxxxxxxx>
> 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.