Re: [Qemu-devel] [PATCH] ivshmem: use error_report

[Andrew Jones]

On Thu, Sep 18, 2014 at 04:46:55PM -0600, Eric Blake wrote:
> On 09/18/2014 04:39 PM, Andrew Jones wrote:
> > Replace all the fprintf(stderr, ...) calls with error_report.
> > 
> > Signed-off-by: Andrew Jones <address@hidden>
> > ---
> >  hw/misc/ivshmem.c | 27 +++++++++++++--------------
> >  1 file changed, 13 insertions(+), 14 deletions(-)
> > 
> > diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> > index bf585b7691998..d285df7d65a9f 100644
> > --- a/hw/misc/ivshmem.c
> > +++ b/hw/misc/ivshmem.c
> > @@ -300,7 +300,7 @@ static CharDriverState* create_eventfd_chr_device(void 
> > * opaque, EventNotifier *
> >      chr = qemu_chr_open_eventfd(eventfd);
> >  
> >      if (chr == NULL) {
> > -        fprintf(stderr, "creating eventfd for eventfd %d failed\n", 
> > eventfd);
> > +        error_report("creating eventfd for eventfd %d failed\n", eventfd);
> 
> The conversion to error_report() should also drop trailing \n.

Thanks for the quick review. I'll send a v2 with this change.

> 
> >          exit(-1);
> 
> Another bug (but probably worth cleaning up in a separate patch) -
> exit(-1) is the same as exit(255), which is not a usual exit status
> (although it DOES make xargs behave differently).

I'll go ahead an squeeze a s/-1/1/ exit code change into this patch
too. I think it's a similar enough cleanup.

> 
> 
> >      /* BARs must be a power of 2 */
> >      if (!is_power_of_two(value)) {
> > -        fprintf(stderr, "ivshmem: size must be power of 2\n");
> > +        error_report("size must be power of 2\n");
> >          exit(1);
> 
> But seeing as how much of this file uses the more typical exit(1), we
> should consistently use 1 in all places where we exit early.
> 
> >          } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR,
> >                          S_IRWXU|S_IRWXG|S_IRWXO)) < 0) {
> > -            fprintf(stderr, "ivshmem: could not open shared file\n");
> > +            error_report("could not open shared file\n");
> >              exit(-1);
> 
> Another weird use of exit(-1).
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>