On Wed, Feb 26, 2020 at 09:10:30PM +0100, Philippe Mathieu-Daudé wrote: > The vtd_find_as_from_bus_num() function was introduced (in commit > dbaabb25f) in a code format that could return an incorrect pointer, > which was later fixed by commit a2e1cd41ccf. > We could have avoid this by writing the if() statement differently. > Do it now, in case this function is re-used. The code is easier to > review (harder to miss bugs). > > Signed-off-by: Philippe Mathieu-Daudé <[email protected]> > --- > Since v2: Re-worded commit description, patch sent without -w. > > This patch is easier to review with 'git-diff -w' (--ignore-all-space): > > @@ -987,14 +987,17 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, > uint32_t level) > static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState *s, uint8_t bus_num) > { > VTDBus *vtd_bus = s->vtd_as_by_bus_num[bus_num]; > - if (!vtd_bus) { > + GHashTableIter iter; > + > + if (vtd_bus) { > + return vtd_bus; > + } > + > /* > * Iterate over the registered buses to find the one which > * currently hold this bus number, and update the bus_num > * lookup table: > */ > - GHashTableIter iter; > - > g_hash_table_iter_init(&iter, s->vtd_as_by_busptr); > while (g_hash_table_iter_next(&iter, NULL, (void **)&vtd_bus)) { > if (pci_bus_num(vtd_bus->bus) == bus_num) { > @@ -1002,9 +1005,8 @@ static VTDBus *vtd_find_as_from_bus_num(IntelIOMMUState > *s, uint8_t bus_num) > return vtd_bus; > } > } > - vtd_bus = NULL; > - } > - return vtd_bus; > + > + return NULL; > }
(However we'll need to read below after all... :) Reviewed-by: Peter Xu <[email protected]> -- Peter Xu
