Re: [PATCH] xen/arm: pci: fix -Wtype-limits warning in pci-host-common.c

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Stewart,

> On 3 May 2023, at 21:18, Stewart Hildebrand <Stewart.Hildebrand@xxxxxxx> 
> wrote:
> 
> When building with EXTRA_CFLAGS_XEN_CORE="-Wtype-limits", we observe the
> following warning:
> 
> arch/arm/pci/pci-host-common.c: In function ‘pci_host_common_probe’:
> arch/arm/pci/pci-host-common.c:238:26: warning: comparison is always false 
> due to limited range of data type [-Wtype-limits]
>  238 |     if ( bridge->segment < 0 )
>      |                          ^
> 
> This is due to bridge->segment being an unsigned type. Fix it by introducing a
> new variable of signed type to use in the condition.
> 
> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@xxxxxxx>

I would see this as a bug fix more than a compiler warning fix as the error 
code was
ignored before that.

Anyway:
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand


> ---
> xen/arch/arm/pci/pci-host-common.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/pci/pci-host-common.c 
> b/xen/arch/arm/pci/pci-host-common.c
> index a8ece94303ca..7474d877deb8 100644
> --- a/xen/arch/arm/pci/pci-host-common.c
> +++ b/xen/arch/arm/pci/pci-host-common.c
> @@ -214,6 +214,7 @@ int pci_host_common_probe(struct dt_device_node *dev,
>     struct pci_host_bridge *bridge;
>     struct pci_config_window *cfg;
>     int err;
> +    int domain;
> 
>     if ( dt_device_for_passthrough(dev) )
>         return 0;
> @@ -234,12 +235,13 @@ int pci_host_common_probe(struct dt_device_node *dev,
>     bridge->cfg = cfg;
>     bridge->ops = &ops->pci_ops;
> 
> -    bridge->segment = pci_bus_find_domain_nr(dev);
> -    if ( bridge->segment < 0 )
> +    domain = pci_bus_find_domain_nr(dev);
> +    if ( domain < 0 )
>     {
>         printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" property in 
> DT\n");
>         BUG();
>     }
> +    bridge->segment = domain;
>     pci_add_host_bridge(bridge);
> 
>     return 0;
> -- 
> 2.40.1
>