On Thu, Jan 25, 2024 at 10:16:55 -0800, Andrea Bolognani wrote:
> On Thu, Jan 25, 2024 at 11:17:46AM +0100, Peter Krempa wrote:
> > On Thu, Jan 25, 2024 at 11:15:14 +0100, Peter Krempa wrote:
> > > On Wed, Jan 24, 2024 at 20:37:33 +0100, Andrea Bolognani wrote:
> > > > @@ -5584,7 +5584,9 @@ 
> > > > qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
> > > >      switch (cont->type) {
> > > >      case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> > > >          /* Set the default SCSI controller model if not already set */
> > > > -        if (qemuDomainSetSCSIControllerModel(def, cont, qemuCaps) < 0)
> > > > +        cont->model = qemuDomainGetSCSIControllerModel(def, cont, 
> > > > qemuCaps);
> > > > +
> > > > +        if (cont->model < 0)
> > > >              return -1;
> > >
> > > This is not future proof for the case when 'model' gets turned into a
> > > proper enum.
> >
> > Based on further patches, it seems to be a problem in more places.
> 
> I'm not sure I understand. Can you please explain what "a proper
> enum" means in this context? Is that connected to some condition that

If you turn the 'model' member from 'int' to the appropriate enum type
the '< 0' check will no longer work if the enum has no negative members.
This is due to the fact that the compiler then treats the enum as
'unsigned'

> could present itself in the current code, or as a consequence of a
> potential future refactoring? Based on the fact that you ultimately
> ACKed the patch, I assume the latter.

There are multiple other instances of exactly the same problem for this
enum and additionally this one has a native -1 member, so my point was
moot mostly.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

Reply via email to