[PATCH 5/7] vmx: Rework virVMXConfigScanResultsCollector slightly

[Peter Krempa]

On Wed, Nov 16, 2022 at 12:37:30 +0100, Michal Prívozník wrote:
> On 11/16/22 12:35, Peter Krempa wrote:
> > On Wed, Nov 16, 2022 at 10:11:43 +0100, Michal Privoznik wrote:
> >> The idea here is that virVMXConfigScanResultsCollector() sets the
> >> networks_max_index to the highest ethernet index seen. Well, the
> >> struct member is signed int, we parse just seen index into uint
> >> and then typecast to compare the two. This is not necessary,
> >> because the maximum number of NICs a vSphere domain can have is
> >> (<drumrolll/>): ten [1]. This will fit into signed int easily
> >> anywhere.
> >>
> >> 1: https://configmax.esp.vmware.com/guest?vmwareproduct=vSphere&release=vSphere%208.0&categories=1-0
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> >> ---
> >>  src/vmx/vmx.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> >> index d3e452e3ef..287a877b4a 100644
> >> --- a/src/vmx/vmx.c
> >> +++ b/src/vmx/vmx.c
> >> @@ -1324,10 +1324,10 @@ virVMXConfigScanResultsCollector(const char* name,
> >>      const char *suffix = NULL;
> >>  
> >>      if ((suffix = STRCASESKIP(name, "ethernet"))) {
> >> -        unsigned int idx;
> >> +        int idx;
> >>          char *p;
> >>  
> >> -        if (virStrToLong_uip(suffix, &p, 10, &idx) < 0 ||
> >> +        if (virStrToLong_i(suffix, &p, 10, &idx) < 0 ||
> >>              *p != '.') {
> >>              virReportError(VIR_ERR_INTERNAL_ERROR,
> >>                             _("failed to parse the index of the VMX key '%s'"),
> > 
> > Just a note, because it isn't obvious neiter from the context, nor from
> > the commit message.
> > 
> > 'virStrToLong_uip' validates that the parsed number is not negative.
> > 
> > Switching to virStrToLong_i allows negative numbers to be considered,
> > but in this case it won't matter too much. We'd just ignore the network
> > device if the index for some reason was -1, and additionally we'd trust
> > any massive positive number anyways.
> > 
> 
> Yeah, do you want me to put idx < 0 check in or mention this in a commit
> message?

I don't think it will be a problem even without any change, but you can
add the idx < 0 check to the existing *p != '.' to preserve the
semantics completely.