Marc,
Sorry for the previous reply. My email settings were not correct, so it
inserted those confidentiality text, which was not what I intended.
This is what I think:
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ced10c4..0b60bb0 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1284,7 +1284,7 @@ static bool gic_check_eoimode(struct device_node
*node, void __iomem **base)
{
struct resource cpuif_res;
- of_address_to_resource(node, 1, &cpuif_res);
+ (void)of_address_to_resource(node, 1, &cpuif_res);
if (!is_hyp_mode_available())
return false;
We are 100% sure of_address_to_resource will succeed in this particular
case, so "(void)" will help suppress Coverity warning.
On 07/05/2018 12:18 PM, Bo Yan wrote:
Marc,
I'm also wondering if of_address_to_resource can really fail in this
particular case?
What if we just explicitly discard the return value like this:
(void)of_address_to_resource(node, 1, &cpuif_res);
This suppresses Coverity warning by explicitly stating we are 100% sure
the function call will always return success.
On 07/05/2018 12:13 PM, Marc Zyngier wrote:
Hi Bo,
On Thu, 5 Jul 2018 11:20:59 -0700
Bo Yan <[email protected]> wrote:
The of_address_to_resource returns 0 if successful. gic_check_eoimode
calls it without checking the return value. This induces Coverity
warning: "Unchecked return value".
Return false from gic_check_eoimode if of_address_to_resource returns
non-0 value.
Signed-off-by: Bo Yan <[email protected]>
---
drivers/irqchip/irq-gic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index ced10c4..0bceb10 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1284,7 +1284,8 @@ static bool gic_check_eoimode(struct
device_node *node, void __iomem **base)
{
struct resource cpuif_res;
- of_address_to_resource(node, 1, &cpuif_res);
+ if (of_address_to_resource(node, 1, &cpuif_res))
+ return false;
We've just done an of_iomap() on this resource, which succeeded. How
can the same thing now fail? It would mean that the device tree has
been pulled from under our feet...
And if it could happen, why is returning false the right thing to do?
Why would we say we want EOImode==0 instead of 1?
if (!is_hyp_mode_available())
return false;
As it stands, I'm not taking such a patch. It either papers over a
bigger problem, or just keeps a warning quiet for the sake of it.
Thanks,
M.
