Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 20 Oct 2018 01:38:06 +0200
From:   Marcin Ciupak <marcin.s.ciupak@gmail.com>
To:     Dan Carpenter <dan.carpenter@oracle.com>
Cc:     kbuild@01.org, kbuild-all@01.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Joe Perches <joe@perches.com>, devel@driverdev.osuosl.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] staging: nrf24: add new driver for 2.4GHz radio
 transceiver
Message-ID: <20181019233806.tzwn7dv63nzbepgl@metis.ciupak.eu>
References: <20181017110001.65pcxebwsirsclim@metis.ciupak.eu>
 <20181019065801.vp6nmaodt7n7qyqh@mwanda>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181019065801.vp6nmaodt7n7qyqh@mwanda>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Oct 19, 2018 at 09:58:01AM +0300, Dan Carpenter wrote:
> Hi Marcin,
> 
> Thank you for the patch! Perhaps something to improve:
> 
> url:    https://github.com/0day-ci/linux/commits/Marcin-Ciupak/staging-nrf24-add-new-driver-for-2-4GHz-radio-transceiver/20181018-164230
> 
> smatch warnings:
> drivers/staging/nrf24/nrf24_if.c:538 nrf24_create_pipe() error: 'p->dev' dereferencing possible ERR_PTR()
> drivers/staging/nrf24/nrf24_if.c:753 nrf24_probe() error: 'device' dereferencing possible ERR_PTR()
> drivers/staging/nrf24/nrf24_sysfs.c:118 plw_store() warn: impossible condition '(old < 0) => (0-255 < 0)'
> 
> # https://github.com/0day-ci/linux/commit/f4952b4a228f4dc0149a814630c94e0e87a9f473
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout f4952b4a228f4dc0149a814630c94e0e87a9f473
> vim +538 drivers/staging/nrf24/nrf24_if.c
> 
> f4952b4a Marcin Ciupak 2018-10-17  503  
> f4952b4a Marcin Ciupak 2018-10-17  504  static struct nrf24_pipe *nrf24_create_pipe(struct nrf24_device *device, int id)
> f4952b4a Marcin Ciupak 2018-10-17  505  {
> f4952b4a Marcin Ciupak 2018-10-17  506  	int ret;
> f4952b4a Marcin Ciupak 2018-10-17  507  	struct nrf24_pipe *p;
> f4952b4a Marcin Ciupak 2018-10-17  508  
> f4952b4a Marcin Ciupak 2018-10-17  509  	//sets flags to false as well
> f4952b4a Marcin Ciupak 2018-10-17  510  	p = kzalloc(sizeof(*p), GFP_KERNEL);
> f4952b4a Marcin Ciupak 2018-10-17  511  	if (!p) {
> f4952b4a Marcin Ciupak 2018-10-17  512  		ret = -ENOMEM;
> f4952b4a Marcin Ciupak 2018-10-17  513  		goto err_return;
> f4952b4a Marcin Ciupak 2018-10-17  514  	}
> f4952b4a Marcin Ciupak 2018-10-17  515  
> f4952b4a Marcin Ciupak 2018-10-17  516  	ret = ida_simple_get(&nrf24_ida_pipe, 0, 0, GFP_KERNEL);
> f4952b4a Marcin Ciupak 2018-10-17  517  	if (ret < 0) {
> f4952b4a Marcin Ciupak 2018-10-17  518  		dev_err(&device->dev, "%s: get_minor failed\n", __func__);
> f4952b4a Marcin Ciupak 2018-10-17  519  		goto err_free_mem;
> f4952b4a Marcin Ciupak 2018-10-17  520  	}
> f4952b4a Marcin Ciupak 2018-10-17  521  
> f4952b4a Marcin Ciupak 2018-10-17  522  	p->devt = MKDEV(MAJOR(nrf24_dev), ret);
> f4952b4a Marcin Ciupak 2018-10-17  523  	p->id = id;
> f4952b4a Marcin Ciupak 2018-10-17  524  
> f4952b4a Marcin Ciupak 2018-10-17  525  	INIT_KFIFO(p->rx_fifo);
> f4952b4a Marcin Ciupak 2018-10-17  526  	init_waitqueue_head(&p->poll_wait_queue);
> f4952b4a Marcin Ciupak 2018-10-17  527  
> f4952b4a Marcin Ciupak 2018-10-17  528  	p->dev = device_create_with_groups(nrf24_class,
> f4952b4a Marcin Ciupak 2018-10-17  529  					   &device->dev,
> f4952b4a Marcin Ciupak 2018-10-17  530  					   p->devt,
> f4952b4a Marcin Ciupak 2018-10-17  531  					   p,
> f4952b4a Marcin Ciupak 2018-10-17  532  					   nrf24_pipe_groups,
> f4952b4a Marcin Ciupak 2018-10-17  533  					   "%s.%d",
> f4952b4a Marcin Ciupak 2018-10-17  534  					   dev_name(&device->dev),
> f4952b4a Marcin Ciupak 2018-10-17  535  					   id);
> f4952b4a Marcin Ciupak 2018-10-17  536  
> f4952b4a Marcin Ciupak 2018-10-17  537  	if (IS_ERR(p->dev)) {
> f4952b4a Marcin Ciupak 2018-10-17 @538  		dev_err(&device->dev,
> f4952b4a Marcin Ciupak 2018-10-17  539  			"%s: device_create of '%s' failed\n",
> f4952b4a Marcin Ciupak 2018-10-17  540  			__func__,
> f4952b4a Marcin Ciupak 2018-10-17  541  			dev_name(p->dev));
> f4952b4a Marcin Ciupak 2018-10-17  542  		ret = PTR_ERR(p->dev);
> f4952b4a Marcin Ciupak 2018-10-17  543  		goto err_ida_remove;
> f4952b4a Marcin Ciupak 2018-10-17  544  	}
> f4952b4a Marcin Ciupak 2018-10-17  545  
> f4952b4a Marcin Ciupak 2018-10-17  546  	cdev_init(&p->cdev, &nrf24_fops);
> f4952b4a Marcin Ciupak 2018-10-17  547  	p->cdev.owner = THIS_MODULE;
> f4952b4a Marcin Ciupak 2018-10-17  548  	ret = cdev_add(&p->cdev, p->devt, 1);
> f4952b4a Marcin Ciupak 2018-10-17  549  	if (ret < 0) {
> f4952b4a Marcin Ciupak 2018-10-17  550  		dev_err(&device->dev, "%s: cdev failed\n", __func__);
> f4952b4a Marcin Ciupak 2018-10-17  551  		goto err_dev_destroy;
> f4952b4a Marcin Ciupak 2018-10-17  552  	}
> f4952b4a Marcin Ciupak 2018-10-17  553  
> f4952b4a Marcin Ciupak 2018-10-17  554  	dev_dbg(&device->dev,
> f4952b4a Marcin Ciupak 2018-10-17  555  		"%s: device created: major(%d), minor(%d)\n",
> f4952b4a Marcin Ciupak 2018-10-17  556  		__func__,
> f4952b4a Marcin Ciupak 2018-10-17  557  		MAJOR(p->devt),
> f4952b4a Marcin Ciupak 2018-10-17  558  		MINOR(p->devt));
> f4952b4a Marcin Ciupak 2018-10-17  559  
> f4952b4a Marcin Ciupak 2018-10-17  560  	return p;
> f4952b4a Marcin Ciupak 2018-10-17  561  
> f4952b4a Marcin Ciupak 2018-10-17  562  err_dev_destroy:
> f4952b4a Marcin Ciupak 2018-10-17  563  	device_destroy(nrf24_class, p->devt);
> f4952b4a Marcin Ciupak 2018-10-17  564  err_ida_remove:
> f4952b4a Marcin Ciupak 2018-10-17  565  	ida_simple_remove(&nrf24_ida_pipe, MINOR(p->devt));
> f4952b4a Marcin Ciupak 2018-10-17  566  err_free_mem:
> f4952b4a Marcin Ciupak 2018-10-17  567  	kfree(p);
> f4952b4a Marcin Ciupak 2018-10-17  568  err_return:
> f4952b4a Marcin Ciupak 2018-10-17  569  	return ERR_PTR(ret);
> f4952b4a Marcin Ciupak 2018-10-17  570  }
> f4952b4a Marcin Ciupak 2018-10-17  571  
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Interesting, I will take a look on that (and most probably provide a new
patch)

Thanks for the hint Dan!

br,
Marcin