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@xxxxxxxxxx> > >> --- > >> 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.
