Re: [PATCH] mm: hugetlb: checking for IS_ERR() instead of NULL

[Nigel Christian <nigel.l.christian@gmail.com>]

On Wed, Jun 02, 2021 at 05:22:24PM +0300, Dan Carpenter wrote:
> On Tue, Jun 01, 2021 at 10:51:23PM +0300, Dan Carpenter wrote:
> > Here is my next attempt at this check.
> > 
> > Back in 2009, I used to write Smatch checks which were too complicated.
> > Ideally, each Smatch check should only print one warning.  The state
> > engine should only have one custom state, and &undefined and &merged.
> > That check I sent violated all those rules.
> > 
> > The other thing which might be interesting is if you pass a NULL
> > to IS_ERR() and then dereference the NULL then print a warning about
> > that.  This has a lot of overlaps with some of my existing checks, but
> > it's still a new idea so it belongs in a separate check.  It's fine and
> > good even if one bug triggers a lot of different warnings.  I'll write
> > that, hang on, brb.
> > 
> > regards,
> > dan carpenter
> > 
> 
> This check worked decently enough.  If you want to fix some of the bugs
> here they are.  I'll look at them in a couple weeks.  I fixed a couple
> of the first ones I looked at (not listed).

Yes, for sure! I'll get started from the top of the list below (^_^)

> 
> drivers/phy/microchip/sparx5_serdes.c:2474 sparx5_serdes_probe() warn: 'iomem' is not an error pointer
> drivers/usb/musb/musb_core.c:2483 musb_init_controller() warn: 'musb->dma_controller' is not an error pointer
> drivers/base/power/domain.c:2566 genpd_dev_pm_detach() warn: 'pd' is not an error pointer
> drivers/base/power/domain.c:2599 genpd_dev_pm_sync() warn: 'pd' is not an error pointer
> drivers/pci/controller/pci-ftpci100.c:496 faraday_pci_probe() warn: 'p->bus_clk' is not an error pointer
> drivers/infiniband/core/cm.c:2348 ib_send_cm_rtu() warn: 'data' is not an error pointer
> drivers/infiniband/core/cm.c:2761 ib_send_cm_drep() warn: 'data' is not an error pointer
> drivers/infiniband/core/cm.c:3092 ib_send_cm_mra() warn: 'data' is not an error pointer
> drivers/bluetooth/btqcomsmd.c:140 btqcomsmd_probe() warn: 'btq->acl_channel' is not an error pointer
> drivers/bluetooth/btqcomsmd.c:145 btqcomsmd_probe() warn: 'btq->cmd_channel' is not an error pointer
> drivers/media/platform/sti/bdisp/bdisp-v4l2.c:1402 bdisp_probe() warn: 'bdisp->clock' is not an error pointer
> drivers/net/ipa/ipa_modem.c:360 ipa_modem_config() warn: 'notifier' is not an error pointer
> drivers/net/wireless/ath/wcn36xx/main.c:1411 wcn36xx_probe() warn: 'wcn->smd_channel' is not an error pointer
> drivers/net/can/spi/hi311x.c:854 hi3110_can_probe() warn: 'clk' is not an error pointer
> drivers/net/can/spi/hi311x.c:941 hi3110_can_probe() warn: 'clk' is not an error pointer
> net/bridge/br_forward.c:223 br_flood() warn: 'prev' is not an error pointer
> net/bridge/br_forward.c:313 br_multicast_flood() warn: 'prev' is not an error pointer
> 
> One thing that I realized is that for functions the return NULL when
> they are configured out like media_device_usb_allocate() is that these
> are always a one liner:

Interesting. 
> 
> struct foo *whatever(void) { return NULL; }
> 
> And they're always in the .h file so we have access to them and can add
> a check for that.
> 
> static bool is_one_liner_function(struct expression *fn)
> {
>         struct symbol *sym;
>         int lines;
> 
>         if (fn->type != EXPR_SYMBOL || !fn->symbol)
>                 return false;
>         sym = get_base_type(fn->symbol);
>         if (!sym)
>                 return false;
>         if (sym->stmt && sym->stmt->type == STMT_COMPOUND)
>                 lines = ptr_list_size((struct ptr_list *)sym->stmt->stmts);
>         else if (sym->inline_stmt && sym->inline_stmt->type == STMT_COMPOUND)
>                 lines = ptr_list_size((struct ptr_list *)sym->inline_stmt->stmts);
>         else
>                 return false;
> 
>         if (lines == 1)
>                 return true;
> 
>         return false;
> }
> 
> regards,
> dan carpenter