Messages in this thread |  | | From | Yejune Deng <> | Date | Wed, 30 Dec 2020 10:16:14 +0800 | Subject | Re: [PATCH] irqchip/gic: add WARN_ON() to facilitate backtracking |
| |
OK,I will adopt it and resubmit.
On Tue, Dec 29, 2020 at 5:27 PM Marc Zyngier <maz@kernel.org> wrote: > > Yejune, > > On 2020-12-29 07:15, Yejune Deng wrote: > > There is two function gic_of_init() and gic_of_init_child() called > > gic_of_setup(),so add WARN_ON() to facilitate backtracking. > > > > Signed-off-by: Yejune Deng <yejune.deng@gmail.com> > > --- > > drivers/irqchip/irq-gic.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c > > index b1d9c22..7c11705 100644 > > --- a/drivers/irqchip/irq-gic.c > > +++ b/drivers/irqchip/irq-gic.c > > @@ -1380,7 +1380,7 @@ static bool gic_check_eoimode(struct device_node > > *node, void __iomem **base) > > > > static int gic_of_setup(struct gic_chip_data *gic, struct device_node > > *node) > > { > > - if (!gic || !node) > > + if (WARN_ON(!gic || !node)) > > return -EINVAL; > > > > gic->raw_dist_base = of_iomap(node, 0); > > I don't immediately see what you gain with this. If you end-up here > with NULL pointers, that either because: > > - you have failed to allocate the GIC private data structure: > but as it turns out, these allocations either cannot fail (gic_data[] > is static), or the dynamic allocation in gic_of_init_child() is > already > checked. > > - node is NULL: both paths already check for a NULL node, so that cannot > fail either. > > My conclusion is that these checks can never trigger, and we should be > able to *remove* them altogether. > > Am I missing something? > > M. > -- > Jazz is not dead. It just smells funny...
|  |