On Thu, May 14, 2020 at 06:09:46PM +0100, Dr. David Alan Gilbert wrote:
> * David Gibson (david@gibson.dropbear.id.au) wrote:
[snip]
> > @@ -649,20 +649,20 @@ static int sev_kvm_init(GuestMemoryProtection *gmpo)
> >      devname = object_property_get_str(OBJECT(sev), "sev-device", NULL);
> >      sev->sev_fd = open(devname, O_RDWR);
> >      if (sev->sev_fd < 0) {
> > -        error_report("%s: Failed to open %s '%s'", __func__,
> > -                     devname, strerror(errno));
> > -    }
> > -    g_free(devname);
> > -    if (sev->sev_fd < 0) {
> > +        g_free(devname);
> > +        error_setg(errp, "%s: Failed to open %s '%s'", __func__,
> > +                   devname, strerror(errno));
> > +        g_free(devname);
> 
> You seem to have double free'd devname - would g_autofree work here?

Oops, fixed.  I'm not really familiar with the g_autofree stuff as
yet, so, maybe?

I also entirely forgot to write a non-placeholder commit message for
this patch.  Oops.

> other than that, looks OK to me.



> 
> Dave
> 
> >          goto err;
> >      }
> > +    g_free(devname);
> >  
> >      ret = sev_platform_ioctl(sev->sev_fd, SEV_PLATFORM_STATUS, &status,
> >                               &fw_error);
> >      if (ret) {
> > -        error_report("%s: failed to get platform status ret=%d "
> > -                     "fw_error='%d: %s'", __func__, ret, fw_error,
> > -                     fw_error_to_str(fw_error));
> > +        error_setg(errp, "%s: failed to get platform status ret=%d "
> > +                   "fw_error='%d: %s'", __func__, ret, fw_error,
> > +                   fw_error_to_str(fw_error));
> >          goto err;
> >      }
> >      sev->build_id = status.build;
> > @@ -672,14 +672,14 @@ static int sev_kvm_init(GuestMemoryProtection *gmpo)
> >      trace_kvm_sev_init();
> >      ret = sev_ioctl(sev->sev_fd, KVM_SEV_INIT, NULL, &fw_error);
> >      if (ret) {
> > -        error_report("%s: failed to initialize ret=%d fw_error=%d '%s'",
> > -                     __func__, ret, fw_error, fw_error_to_str(fw_error));
> > +        error_setg(errp, "%s: failed to initialize ret=%d fw_error=%d '%s'",
> > +                   __func__, ret, fw_error, fw_error_to_str(fw_error));
> >          goto err;
> >      }
> >  
> >      ret = sev_launch_start(sev);
> >      if (ret) {
> > -        error_report("%s: failed to create encryption context", __func__);
> > +        error_setg(errp, "%s: failed to create encryption context", __func__);
> >          goto err;
> >      }
> >  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson