Re: [Xen-devel] [PATCH] AMD IOMMU: correctly propagate errors from amd_iommu_init()

["Xu, Quan" <quan.xu@xxxxxxxxx>]

On June 16, 2016 4:29 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>> On 16.06.16 at 04:03, <quan.xu@xxxxxxxxx> wrote:
> > On June 14, 2016 5:03 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >> -    if ( amd_iommu_update_ivrs_mapping_acpi() != 0 )
> >> +    rc = amd_iommu_update_ivrs_mapping_acpi();
> >> +    if ( rc )
> >>          goto error_out;
> >>
> >>      /* initialize io-apic interrupt remapping entries */
> >> -    if ( iommu_intremap && amd_iommu_setup_ioapic_remapping() != 0 )
> >> +    if ( iommu_intremap )
> >> +        rc = amd_iommu_setup_ioapic_remapping();
> >> +    if ( rc )
> >>          goto error_out;
> >
> >
> > Is it better to indent this if() here? Then,
> >
> > +    if ( iommu_intremap )
> > +    {
> > +        rc = amd_iommu_setup_ioapic_remapping();
> > +        if ( rc )
> > +            goto error_out;
> > +    }
> 
> What would this help (apart from increasing LOC and patch size)?
> 

Ah, first of all, it is not a logic issue, but just make the code clear:
      - this if( rc ) is called only if ( iommu_intremap ) is true.
      - this error is from amd_iommu_setup_ioapic_remapping().

Also from '-    if ( iommu_intremap && amd_iommu_setup_ioapic_remapping() != 0 
)', I'd like to fix it as my suggestion.

Quan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel