Re: [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings

0 day kernel build service
 help / color / mirror / Atom feed

* Re: [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
       [not found] <20230514130148.138624-2-prathubaronia2011@gmail.com>
@ 2023-05-15  7:42 ` Dan Carpenter
  2023-05-18 14:31   ` Prathu Baronia
  0 siblings, 1 reply; 25+ messages in thread
From: Dan Carpenter @ 2023-05-15  7:42 UTC (permalink / raw)
  To: oe-kbuild, Prathu Baronia, gregkh
  Cc: lkp, oe-kbuild-all, linux-kernel, linux-staging,
	prathubaronia2011

Hi Prathu,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Prathu-Baronia/axis-fifo-cleanup-space-issues-with-fops-struct/20230514-220201
base:   staging/staging-testing
patch link:    https://lore.kernel.org/r/20230514130148.138624-2-prathubaronia2011%40gmail.com
patch subject: [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
config: i386-randconfig-m021
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202305150358.nt1BkbXz-lkp@intel.com/

New smatch warnings:
drivers/staging/axis-fifo/axis-fifo.c:858 axis_fifo_probe() warn: missing unwind goto?

Old smatch warnings:
drivers/staging/axis-fifo/axis-fifo.c:907 axis_fifo_probe() error: '%pa' expects argument of type 'phys_addr_t*', argument 4 has type 'void**'

vim +858 drivers/staging/axis-fifo/axis-fifo.c

4a965c5f89decd Jacob Feder        2018-07-22  806  static int axis_fifo_probe(struct platform_device *pdev)
4a965c5f89decd Jacob Feder        2018-07-22  807  {
4a965c5f89decd Jacob Feder        2018-07-22  808  	struct resource *r_mem; /* IO mem resources */
4a965c5f89decd Jacob Feder        2018-07-22  809  	struct device *dev = &pdev->dev; /* OS device (from device tree) */
4a965c5f89decd Jacob Feder        2018-07-22  810  	struct axis_fifo *fifo = NULL;
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  811  	char *device_name;
4a965c5f89decd Jacob Feder        2018-07-22  812  	int rc = 0; /* error return value */
4a965c5f89decd Jacob Feder        2018-07-22  813  
4a965c5f89decd Jacob Feder        2018-07-22  814  	/* ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  815  	 *     init wrapper device
4a965c5f89decd Jacob Feder        2018-07-22  816  	 * ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  817  	 */
4a965c5f89decd Jacob Feder        2018-07-22  818  
4a965c5f89decd Jacob Feder        2018-07-22  819  	/* allocate device wrapper memory */
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  820  	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
4a965c5f89decd Jacob Feder        2018-07-22  821  	if (!fifo)
4a965c5f89decd Jacob Feder        2018-07-22  822  		return -ENOMEM;
4a965c5f89decd Jacob Feder        2018-07-22  823  
4a965c5f89decd Jacob Feder        2018-07-22  824  	dev_set_drvdata(dev, fifo);
4a965c5f89decd Jacob Feder        2018-07-22  825  	fifo->dt_device = dev;
4a965c5f89decd Jacob Feder        2018-07-22  826  
4a965c5f89decd Jacob Feder        2018-07-22  827  	init_waitqueue_head(&fifo->read_queue);
4a965c5f89decd Jacob Feder        2018-07-22  828  	init_waitqueue_head(&fifo->write_queue);
4a965c5f89decd Jacob Feder        2018-07-22  829  
0443b3f4436321 Quentin Deslandes  2020-01-21  830  	mutex_init(&fifo->read_lock);
0443b3f4436321 Quentin Deslandes  2020-01-21  831  	mutex_init(&fifo->write_lock);
4a965c5f89decd Jacob Feder        2018-07-22  832  
4a965c5f89decd Jacob Feder        2018-07-22  833  	/* ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  834  	 *   init device memory space
4a965c5f89decd Jacob Feder        2018-07-22  835  	 * ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  836  	 */
4a965c5f89decd Jacob Feder        2018-07-22  837  
4a965c5f89decd Jacob Feder        2018-07-22  838  	/* get iospace for the device */
4a965c5f89decd Jacob Feder        2018-07-22  839  	r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
4a965c5f89decd Jacob Feder        2018-07-22  840  	if (!r_mem) {
4a965c5f89decd Jacob Feder        2018-07-22  841  		dev_err(fifo->dt_device, "invalid address\n");
4a965c5f89decd Jacob Feder        2018-07-22  842  		rc = -ENODEV;
4a965c5f89decd Jacob Feder        2018-07-22  843  		goto err_initial;
4a965c5f89decd Jacob Feder        2018-07-22  844  	}
4a965c5f89decd Jacob Feder        2018-07-22  845  
4a965c5f89decd Jacob Feder        2018-07-22  846  	/* request physical memory */
354e27a86b4c64 Quentin Deslandes  2019-11-01  847  	fifo->base_addr = devm_ioremap_resource(fifo->dt_device, r_mem);
6a20d283ed6867 Quentin Deslandes  2019-11-01  848  	if (IS_ERR(fifo->base_addr)) {
6a20d283ed6867 Quentin Deslandes  2019-11-01  849  		rc = PTR_ERR(fifo->base_addr);
4a965c5f89decd Jacob Feder        2018-07-22  850  		goto err_initial;
4a965c5f89decd Jacob Feder        2018-07-22  851  	}
4a965c5f89decd Jacob Feder        2018-07-22  852  
4a965c5f89decd Jacob Feder        2018-07-22  853  	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
4a965c5f89decd Jacob Feder        2018-07-22  854  
4a965c5f89decd Jacob Feder        2018-07-22  855  	/* create unique device name */
c5cab4f62648eb Prathu Baronia     2023-05-14  856  	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%pa", DRIVER_NAME, &r_mem->start);
c5cab4f62648eb Prathu Baronia     2023-05-14  857  	if (!device_name)
c5cab4f62648eb Prathu Baronia     2023-05-14 @858  		return -ENOMEM;

		rc = -ENOMEM;
		goto err_initial;

4a965c5f89decd Jacob Feder        2018-07-22  859  	dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
4a965c5f89decd Jacob Feder        2018-07-22  860  
4a965c5f89decd Jacob Feder        2018-07-22  861  	/* ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  862  	 *          init IP
4a965c5f89decd Jacob Feder        2018-07-22  863  	 * ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  864  	 */
4a965c5f89decd Jacob Feder        2018-07-22  865  
ed6daf2b2832d9 Quentin Deslandes  2019-11-01  866  	rc = axis_fifo_parse_dt(fifo);
4a965c5f89decd Jacob Feder        2018-07-22  867  	if (rc)
6a20d283ed6867 Quentin Deslandes  2019-11-01  868  		goto err_initial;
4a965c5f89decd Jacob Feder        2018-07-22  869  
4a965c5f89decd Jacob Feder        2018-07-22  870  	reset_ip_core(fifo);
4a965c5f89decd Jacob Feder        2018-07-22  871  
4a965c5f89decd Jacob Feder        2018-07-22  872  	/* ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  873  	 *    init device interrupts
4a965c5f89decd Jacob Feder        2018-07-22  874  	 * ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  875  	 */
4a965c5f89decd Jacob Feder        2018-07-22  876  
4a965c5f89decd Jacob Feder        2018-07-22  877  	/* get IRQ resource */
790ada0e6ec33e Lad Prabhakar      2021-12-24  878  	rc = platform_get_irq(pdev, 0);
790ada0e6ec33e Lad Prabhakar      2021-12-24  879  	if (rc < 0)
6a20d283ed6867 Quentin Deslandes  2019-11-01  880  		goto err_initial;
4a965c5f89decd Jacob Feder        2018-07-22  881  
4a965c5f89decd Jacob Feder        2018-07-22  882  	/* request IRQ */
790ada0e6ec33e Lad Prabhakar      2021-12-24  883  	fifo->irq = rc;
6a20d283ed6867 Quentin Deslandes  2019-11-01  884  	rc = devm_request_irq(fifo->dt_device, fifo->irq, &axis_fifo_irq, 0,
6a20d283ed6867 Quentin Deslandes  2019-11-01  885  			      DRIVER_NAME, fifo);
4a965c5f89decd Jacob Feder        2018-07-22  886  	if (rc) {
4a965c5f89decd Jacob Feder        2018-07-22  887  		dev_err(fifo->dt_device, "couldn't allocate interrupt %i\n",
4a965c5f89decd Jacob Feder        2018-07-22  888  			fifo->irq);
6a20d283ed6867 Quentin Deslandes  2019-11-01  889  		goto err_initial;
4a965c5f89decd Jacob Feder        2018-07-22  890  	}
4a965c5f89decd Jacob Feder        2018-07-22  891  
4a965c5f89decd Jacob Feder        2018-07-22  892  	/* ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  893  	 *      init char device
4a965c5f89decd Jacob Feder        2018-07-22  894  	 * ----------------------------
4a965c5f89decd Jacob Feder        2018-07-22  895  	 */
4a965c5f89decd Jacob Feder        2018-07-22  896  
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  897  	/* create character device */
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  898  	fifo->miscdev.fops = &fops;
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  899  	fifo->miscdev.minor = MISC_DYNAMIC_MINOR;
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  900  	fifo->miscdev.name = device_name;
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  901  	fifo->miscdev.groups = axis_fifo_attrs_groups;
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  902  	fifo->miscdev.parent = dev;
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  903  	rc = misc_register(&fifo->miscdev);
4a965c5f89decd Jacob Feder        2018-07-22  904  	if (rc < 0)
6a20d283ed6867 Quentin Deslandes  2019-11-01  905  		goto err_initial;
4a965c5f89decd Jacob Feder        2018-07-22  906  
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  907  	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
                                                                                                                      ^^^^^
d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  908  		 &r_mem->start, &fifo->base_addr, fifo->irq);
                                                                                ^^^^^^^^^^^^^^^^
I think Smatch is correct that it should just be %p instead of %pa but
I don't know the rules here too well.

4a965c5f89decd Jacob Feder        2018-07-22  909  
4a965c5f89decd Jacob Feder        2018-07-22  910  	return 0;
4a965c5f89decd Jacob Feder        2018-07-22  911  
4a965c5f89decd Jacob Feder        2018-07-22  912  err_initial:
4a965c5f89decd Jacob Feder        2018-07-22  913  	dev_set_drvdata(dev, NULL);
4a965c5f89decd Jacob Feder        2018-07-22  914  	return rc;
4a965c5f89decd Jacob Feder        2018-07-22  915  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-15  7:42 ` [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Dan Carpenter
@ 2023-05-18 14:31   ` Prathu Baronia
  2023-05-18 14:51     ` [PATCH v3 " Prathu Baronia
  0 siblings, 1 reply; 25+ messages in thread
From: Prathu Baronia @ 2023-05-18 14:31 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: oe-kbuild, gregkh, lkp, oe-kbuild-all, linux-kernel,
	linux-staging

Hi Dan,

Thanks for pointing these out. The older warning seems to be
introduced in an earlier commit. I will fix both of these in v3.
---
- Prathu


On Mon, May 15, 2023 at 1:13 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hi Prathu,
>
> kernel test robot noticed the following build warnings:
>
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Prathu-Baronia/axis-fifo-cleanup-space-issues-with-fops-struct/20230514-220201
> base:   staging/staging-testing
> patch link:    https://lore.kernel.org/r/20230514130148.138624-2-prathubaronia2011%40gmail.com
> patch subject: [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
> config: i386-randconfig-m021
> compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <error27@gmail.com>
> | Link: https://lore.kernel.org/r/202305150358.nt1BkbXz-lkp@intel.com/
>
> New smatch warnings:
> drivers/staging/axis-fifo/axis-fifo.c:858 axis_fifo_probe() warn: missing unwind goto?
>
> Old smatch warnings:
> drivers/staging/axis-fifo/axis-fifo.c:907 axis_fifo_probe() error: '%pa' expects argument of type 'phys_addr_t*', argument 4 has type 'void**'
>
> vim +858 drivers/staging/axis-fifo/axis-fifo.c
>
> 4a965c5f89decd Jacob Feder        2018-07-22  806  static int axis_fifo_probe(struct platform_device *pdev)
> 4a965c5f89decd Jacob Feder        2018-07-22  807  {
> 4a965c5f89decd Jacob Feder        2018-07-22  808       struct resource *r_mem; /* IO mem resources */
> 4a965c5f89decd Jacob Feder        2018-07-22  809       struct device *dev = &pdev->dev; /* OS device (from device tree) */
> 4a965c5f89decd Jacob Feder        2018-07-22  810       struct axis_fifo *fifo = NULL;
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  811       char *device_name;
> 4a965c5f89decd Jacob Feder        2018-07-22  812       int rc = 0; /* error return value */
> 4a965c5f89decd Jacob Feder        2018-07-22  813
> 4a965c5f89decd Jacob Feder        2018-07-22  814       /* ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  815        *     init wrapper device
> 4a965c5f89decd Jacob Feder        2018-07-22  816        * ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  817        */
> 4a965c5f89decd Jacob Feder        2018-07-22  818
> 4a965c5f89decd Jacob Feder        2018-07-22  819       /* allocate device wrapper memory */
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  820       fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
> 4a965c5f89decd Jacob Feder        2018-07-22  821       if (!fifo)
> 4a965c5f89decd Jacob Feder        2018-07-22  822               return -ENOMEM;
> 4a965c5f89decd Jacob Feder        2018-07-22  823
> 4a965c5f89decd Jacob Feder        2018-07-22  824       dev_set_drvdata(dev, fifo);
> 4a965c5f89decd Jacob Feder        2018-07-22  825       fifo->dt_device = dev;
> 4a965c5f89decd Jacob Feder        2018-07-22  826
> 4a965c5f89decd Jacob Feder        2018-07-22  827       init_waitqueue_head(&fifo->read_queue);
> 4a965c5f89decd Jacob Feder        2018-07-22  828       init_waitqueue_head(&fifo->write_queue);
> 4a965c5f89decd Jacob Feder        2018-07-22  829
> 0443b3f4436321 Quentin Deslandes  2020-01-21  830       mutex_init(&fifo->read_lock);
> 0443b3f4436321 Quentin Deslandes  2020-01-21  831       mutex_init(&fifo->write_lock);
> 4a965c5f89decd Jacob Feder        2018-07-22  832
> 4a965c5f89decd Jacob Feder        2018-07-22  833       /* ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  834        *   init device memory space
> 4a965c5f89decd Jacob Feder        2018-07-22  835        * ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  836        */
> 4a965c5f89decd Jacob Feder        2018-07-22  837
> 4a965c5f89decd Jacob Feder        2018-07-22  838       /* get iospace for the device */
> 4a965c5f89decd Jacob Feder        2018-07-22  839       r_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> 4a965c5f89decd Jacob Feder        2018-07-22  840       if (!r_mem) {
> 4a965c5f89decd Jacob Feder        2018-07-22  841               dev_err(fifo->dt_device, "invalid address\n");
> 4a965c5f89decd Jacob Feder        2018-07-22  842               rc = -ENODEV;
> 4a965c5f89decd Jacob Feder        2018-07-22  843               goto err_initial;
> 4a965c5f89decd Jacob Feder        2018-07-22  844       }
> 4a965c5f89decd Jacob Feder        2018-07-22  845
> 4a965c5f89decd Jacob Feder        2018-07-22  846       /* request physical memory */
> 354e27a86b4c64 Quentin Deslandes  2019-11-01  847       fifo->base_addr = devm_ioremap_resource(fifo->dt_device, r_mem);
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  848       if (IS_ERR(fifo->base_addr)) {
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  849               rc = PTR_ERR(fifo->base_addr);
> 4a965c5f89decd Jacob Feder        2018-07-22  850               goto err_initial;
> 4a965c5f89decd Jacob Feder        2018-07-22  851       }
> 4a965c5f89decd Jacob Feder        2018-07-22  852
> 4a965c5f89decd Jacob Feder        2018-07-22  853       dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
> 4a965c5f89decd Jacob Feder        2018-07-22  854
> 4a965c5f89decd Jacob Feder        2018-07-22  855       /* create unique device name */
> c5cab4f62648eb Prathu Baronia     2023-05-14  856       device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%pa", DRIVER_NAME, &r_mem->start);
> c5cab4f62648eb Prathu Baronia     2023-05-14  857       if (!device_name)
> c5cab4f62648eb Prathu Baronia     2023-05-14 @858               return -ENOMEM;
>
>                 rc = -ENOMEM;
>                 goto err_initial;
>
> 4a965c5f89decd Jacob Feder        2018-07-22  859       dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
> 4a965c5f89decd Jacob Feder        2018-07-22  860
> 4a965c5f89decd Jacob Feder        2018-07-22  861       /* ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  862        *          init IP
> 4a965c5f89decd Jacob Feder        2018-07-22  863        * ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  864        */
> 4a965c5f89decd Jacob Feder        2018-07-22  865
> ed6daf2b2832d9 Quentin Deslandes  2019-11-01  866       rc = axis_fifo_parse_dt(fifo);
> 4a965c5f89decd Jacob Feder        2018-07-22  867       if (rc)
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  868               goto err_initial;
> 4a965c5f89decd Jacob Feder        2018-07-22  869
> 4a965c5f89decd Jacob Feder        2018-07-22  870       reset_ip_core(fifo);
> 4a965c5f89decd Jacob Feder        2018-07-22  871
> 4a965c5f89decd Jacob Feder        2018-07-22  872       /* ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  873        *    init device interrupts
> 4a965c5f89decd Jacob Feder        2018-07-22  874        * ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  875        */
> 4a965c5f89decd Jacob Feder        2018-07-22  876
> 4a965c5f89decd Jacob Feder        2018-07-22  877       /* get IRQ resource */
> 790ada0e6ec33e Lad Prabhakar      2021-12-24  878       rc = platform_get_irq(pdev, 0);
> 790ada0e6ec33e Lad Prabhakar      2021-12-24  879       if (rc < 0)
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  880               goto err_initial;
> 4a965c5f89decd Jacob Feder        2018-07-22  881
> 4a965c5f89decd Jacob Feder        2018-07-22  882       /* request IRQ */
> 790ada0e6ec33e Lad Prabhakar      2021-12-24  883       fifo->irq = rc;
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  884       rc = devm_request_irq(fifo->dt_device, fifo->irq, &axis_fifo_irq, 0,
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  885                             DRIVER_NAME, fifo);
> 4a965c5f89decd Jacob Feder        2018-07-22  886       if (rc) {
> 4a965c5f89decd Jacob Feder        2018-07-22  887               dev_err(fifo->dt_device, "couldn't allocate interrupt %i\n",
> 4a965c5f89decd Jacob Feder        2018-07-22  888                       fifo->irq);
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  889               goto err_initial;
> 4a965c5f89decd Jacob Feder        2018-07-22  890       }
> 4a965c5f89decd Jacob Feder        2018-07-22  891
> 4a965c5f89decd Jacob Feder        2018-07-22  892       /* ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  893        *      init char device
> 4a965c5f89decd Jacob Feder        2018-07-22  894        * ----------------------------
> 4a965c5f89decd Jacob Feder        2018-07-22  895        */
> 4a965c5f89decd Jacob Feder        2018-07-22  896
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  897       /* create character device */
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  898       fifo->miscdev.fops = &fops;
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  899       fifo->miscdev.minor = MISC_DYNAMIC_MINOR;
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  900       fifo->miscdev.name = device_name;
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  901       fifo->miscdev.groups = axis_fifo_attrs_groups;
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  902       fifo->miscdev.parent = dev;
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  903       rc = misc_register(&fifo->miscdev);
> 4a965c5f89decd Jacob Feder        2018-07-22  904       if (rc < 0)
> 6a20d283ed6867 Quentin Deslandes  2019-11-01  905               goto err_initial;
> 4a965c5f89decd Jacob Feder        2018-07-22  906
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  907       dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
>                                                                                                                       ^^^^^
> d2d7aa53891ea1 Greg Kroah-Hartman 2021-09-07  908                &r_mem->start, &fifo->base_addr, fifo->irq);
>                                                                                 ^^^^^^^^^^^^^^^^
> I think Smatch is correct that it should just be %p instead of %pa but
> I don't know the rules here too well.
>
> 4a965c5f89decd Jacob Feder        2018-07-22  909
> 4a965c5f89decd Jacob Feder        2018-07-22  910       return 0;
> 4a965c5f89decd Jacob Feder        2018-07-22  911
> 4a965c5f89decd Jacob Feder        2018-07-22  912  err_initial:
> 4a965c5f89decd Jacob Feder        2018-07-22  913       dev_set_drvdata(dev, NULL);
> 4a965c5f89decd Jacob Feder        2018-07-22  914       return rc;
> 4a965c5f89decd Jacob Feder        2018-07-22  915  }
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-18 14:31   ` Prathu Baronia
@ 2023-05-18 14:51     ` Prathu Baronia
  2023-05-18 14:51       ` [PATCH v3 2/2] axis-fifo: cleanup space issues with fops struct Prathu Baronia
  2023-05-27  7:35       ` [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg KH
  0 siblings, 2 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-05-18 14:51 UTC (permalink / raw)
  To: prathubaronia2011
  Cc: dan.carpenter, gregkh, linux-kernel, linux-staging, lkp,
	oe-kbuild-all, oe-kbuild, Dan Carpenter

In various places, string buffers of a fixed size are allocated, and
filled using snprintf() with the same fixed size, which is error-prone.

Replace this by calling devm_kasprintf() instead, which always uses the
appropriate size.

Also fix an old smatch warning reported by lkp introduced by commit
d2d7aa53891e. In the mentioned commit we had used "%pa" format specifier
for a void* type and hence smatch complained about its use instead of
"%p".

Fixes: d2d7aa53891e ("staging: axis-fifo: convert to use miscdevice")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://lore.kernel.org/r/202305150358.nt1BkbXz-lkp@intel.com/
Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
V2 -> V3: Fix smatch warnings from kernel test robot
V1 -> V2: Split into logical commits and fix commit message

 drivers/staging/axis-fifo/axis-fifo.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 7a21f2423204..d71bdc6dd961 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -816,10 +816,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	 * ----------------------------
 	 */
 
-	device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!device_name)
-		return -ENOMEM;
-
 	/* allocate device wrapper memory */
 	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
 	if (!fifo)
@@ -857,7 +853,11 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
 
 	/* create unique device name */
-	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
+	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%pa", DRIVER_NAME, &r_mem->start);
+	if (!device_name) {
+		rc = -ENOMEM;
+		goto err_initial;
+	}
 	dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
 
 	/* ----------------------------
@@ -906,8 +906,8 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	if (rc < 0)
 		goto err_initial;
 
-	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
-		 &r_mem->start, &fifo->base_addr, fifo->irq);
+	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%p, irq=%i\n",
+		 &r_mem->start, fifo->base_addr, fifo->irq);
 
 	return 0;
 

base-commit: 4d6d4c7f541d7027beed4fb86eb2c451bd8d6fff
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v3 2/2] axis-fifo: cleanup space issues with fops struct
  2023-05-18 14:51     ` [PATCH v3 " Prathu Baronia
@ 2023-05-18 14:51       ` Prathu Baronia
  2023-05-27  7:35       ` [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg KH
  1 sibling, 0 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-05-18 14:51 UTC (permalink / raw)
  To: prathubaronia2011
  Cc: dan.carpenter, gregkh, linux-kernel, linux-staging, lkp,
	oe-kbuild-all, oe-kbuild

Add required spaces for proper formatting of fops members for better
readability.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index d71bdc6dd961..59e962467a3d 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -716,11 +716,11 @@ static int axis_fifo_close(struct inode *inod, struct file *f)
 }
 
 static const struct file_operations fops = {
-	.owner = THIS_MODULE,
-	.open = axis_fifo_open,
+	.owner   = THIS_MODULE,
+	.open    = axis_fifo_open,
 	.release = axis_fifo_close,
-	.read = axis_fifo_read,
-	.write = axis_fifo_write
+	.read    = axis_fifo_read,
+	.write   = axis_fifo_write
 };
 
 /* read named property from the device tree */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-18 14:51     ` [PATCH v3 " Prathu Baronia
  2023-05-18 14:51       ` [PATCH v3 2/2] axis-fifo: cleanup space issues with fops struct Prathu Baronia
@ 2023-05-27  7:35       ` Greg KH
  2023-05-27  8:53         ` Prathu Baronia
  1 sibling, 1 reply; 25+ messages in thread
From: Greg KH @ 2023-05-27  7:35 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: dan.carpenter, linux-kernel, linux-staging, lkp, oe-kbuild-all,
	oe-kbuild, Dan Carpenter

On Thu, May 18, 2023 at 08:21:53PM +0530, Prathu Baronia wrote:
> In various places, string buffers of a fixed size are allocated, and
> filled using snprintf() with the same fixed size, which is error-prone.

Maybe error-prone, but all is fine with the original code, right?

> Replace this by calling devm_kasprintf() instead, which always uses the
> appropriate size.
> 
> Also fix an old smatch warning reported by lkp introduced by commit
> d2d7aa53891e. In the mentioned commit we had used "%pa" format specifier
> for a void* type and hence smatch complained about its use instead of
> "%p".

When you have "also" in a changelog commit, that usually means this
needs to be split out into a separate patch.  And that's the case here,
make the first patch of the series fix the problem.  Then do your
cleanups on later patches.

> Fixes: d2d7aa53891e ("staging: axis-fifo: convert to use miscdevice")

changing to a different string function does not fix anything.

> Reported-by: kernel test robot <lkp@intel.com>

It did not report that you need to replace a string function, right?

See, things got messy when you mixed in changes into one.  Please break
these up.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-27  7:35       ` [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg KH
@ 2023-05-27  8:53         ` Prathu Baronia
  2023-05-27 11:50           ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Prathu Baronia
  0 siblings, 1 reply; 25+ messages in thread
From: Prathu Baronia @ 2023-05-27  8:53 UTC (permalink / raw)
  To: Greg KH
  Cc: dan.carpenter, linux-kernel, linux-staging, lkp, oe-kbuild-all,
	oe-kbuild, Dan Carpenter

On Sat, May 27, 2023 at 08:35:57AM +0100, Greg KH wrote:
> 
> Maybe error-prone, but all is fine with the original code, right?
> 
Yes, all is fine with the original code. Replaced it only because it was error-prone.

> When you have "also" in a changelog commit, that usually means this
> needs to be split out into a separate patch.  And that's the case here,
> make the first patch of the series fix the problem.  Then do your
> cleanups on later patches.
> 
Point taken, will split it in v4.

> > Fixes: d2d7aa53891e ("staging: axis-fifo: convert to use miscdevice")
> 
> changing to a different string function does not fix anything.
Right, this should only be part of the smatch warning fixing commit.

> 
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> It did not report that you need to replace a string function, right?
> 
> See, things got messy when you mixed in changes into one.  Please break
> these up.
Understood, will do.

Prathu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier
  2023-05-27  8:53         ` Prathu Baronia
@ 2023-05-27 11:50           ` Prathu Baronia
  2023-05-27 11:50             ` [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
                               ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-05-27 11:50 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Uwe Kleine-König,
	Khadija Kamran, Fabio M. De Francesco, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

Fix an old smatch warning reported by lkp introduced by commit
d2d7aa53891e. In the mentioned commit we had used "%pa" format specifier
for a void* type and hence smatch complained about its use instead of
"%p".

Fixes: d2d7aa53891e ("staging: axis-fifo: convert to use miscdevice")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Link: https://lore.kernel.org/r/202305150358.nt1BkbXz-lkp@intel.com/
Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
V3 -> V4: Split into warning fixing and cleanup commits
V2 -> V3: Fix smatch warnings from kernel test robot
V1 -> V2: Split into logical commits and fix commit message

 drivers/staging/axis-fifo/axis-fifo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 7a21f2423204..271cab805cad 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -906,8 +906,8 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	if (rc < 0)
 		goto err_initial;
 
-	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
-		 &r_mem->start, &fifo->base_addr, fifo->irq);
+	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%p, irq=%i\n",
+		 &r_mem->start, fifo->base_addr, fifo->irq);
 
 	return 0;
 

base-commit: 44c026a73be8038f03dbdeef028b642880cf1511
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-27 11:50           ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Prathu Baronia
@ 2023-05-27 11:50             ` Prathu Baronia
  2023-05-28  9:00               ` Greg Kroah-Hartman
  2023-05-27 11:51             ` [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct Prathu Baronia
  2023-05-28  8:59             ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Greg Kroah-Hartman
  2 siblings, 1 reply; 25+ messages in thread
From: Prathu Baronia @ 2023-05-27 11:50 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Uwe Kleine-König,
	Fabio M. De Francesco, Khadija Kamran, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

In various places, string buffers of a fixed size are allocated, and
filled using snprintf() with the same fixed size, which is error-prone.

Replace this by calling devm_kasprintf() instead, which always uses the
appropriate size.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 271cab805cad..d71bdc6dd961 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -816,10 +816,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	 * ----------------------------
 	 */
 
-	device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!device_name)
-		return -ENOMEM;
-
 	/* allocate device wrapper memory */
 	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
 	if (!fifo)
@@ -857,7 +853,11 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
 
 	/* create unique device name */
-	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
+	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%pa", DRIVER_NAME, &r_mem->start);
+	if (!device_name) {
+		rc = -ENOMEM;
+		goto err_initial;
+	}
 	dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
 
 	/* ----------------------------
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-27 11:50             ` [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
@ 2023-05-28  9:00               ` Greg Kroah-Hartman
  2023-06-13 16:03                 ` Prathu Baronia
  0 siblings, 1 reply; 25+ messages in thread
From: Greg Kroah-Hartman @ 2023-05-28  9:00 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: Uwe Kleine-König, Fabio M. De Francesco, Khadija Kamran,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sat, May 27, 2023 at 05:20:59PM +0530, Prathu Baronia wrote:
> In various places, string buffers of a fixed size are allocated, and
> filled using snprintf() with the same fixed size, which is error-prone.
> 
> Replace this by calling devm_kasprintf() instead, which always uses the
> appropriate size.
> 
> Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
> ---
>  drivers/staging/axis-fifo/axis-fifo.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> index 271cab805cad..d71bdc6dd961 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -816,10 +816,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	 * ----------------------------
>  	 */
>  
> -	device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -	if (!device_name)
> -		return -ENOMEM;
> -
>  	/* allocate device wrapper memory */
>  	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
>  	if (!fifo)
> @@ -857,7 +853,11 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
>  
>  	/* create unique device name */
> -	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
> +	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%pa", DRIVER_NAME, &r_mem->start);

we should not be using a kernel address as a device name, please fix
this up to just use a unique number instead.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-05-28  9:00               ` Greg Kroah-Hartman
@ 2023-06-13 16:03                 ` Prathu Baronia
  0 siblings, 0 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-13 16:03 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Uwe Kleine-König, Fabio M. De Francesco, Khadija Kamran,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sun, May 28, 2023 at 10:00:14AM +0100, Greg Kroah-Hartman wrote:
> we should not be using a kernel address as a device name, please fix
> this up to just use a unique number instead.
Agreed, will be fixed in v5.

Prathu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct
  2023-05-27 11:50           ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Prathu Baronia
  2023-05-27 11:50             ` [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
@ 2023-05-27 11:51             ` Prathu Baronia
  2023-05-27 22:31               ` Uwe Kleine-König
  2023-05-28  8:59             ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Greg Kroah-Hartman
  2 siblings, 1 reply; 25+ messages in thread
From: Prathu Baronia @ 2023-05-27 11:51 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Uwe Kleine-König,
	Fabio M. De Francesco, Khadija Kamran, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

Add required spaces for proper formatting of fops members for better
readability.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index d71bdc6dd961..59e962467a3d 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -716,11 +716,11 @@ static int axis_fifo_close(struct inode *inod, struct file *f)
 }
 
 static const struct file_operations fops = {
-	.owner = THIS_MODULE,
-	.open = axis_fifo_open,
+	.owner   = THIS_MODULE,
+	.open    = axis_fifo_open,
 	.release = axis_fifo_close,
-	.read = axis_fifo_read,
-	.write = axis_fifo_write
+	.read    = axis_fifo_read,
+	.write   = axis_fifo_write
 };
 
 /* read named property from the device tree */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct
  2023-05-27 11:51             ` [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct Prathu Baronia
@ 2023-05-27 22:31               ` Uwe Kleine-König
  2023-05-28  8:57                 ` Greg Kroah-Hartman
  2023-06-13 15:56                 ` Prathu Baronia
  0 siblings, 2 replies; 25+ messages in thread
From: Uwe Kleine-König @ 2023-05-27 22:31 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: Greg Kroah-Hartman, Fabio M. De Francesco, Khadija Kamran,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]

Hello,

On Sat, May 27, 2023 at 05:21:00PM +0530, Prathu Baronia wrote:
> Add required spaces for proper formatting of fops members for better
> readability.
> 
> Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
> ---
>  drivers/staging/axis-fifo/axis-fifo.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> index d71bdc6dd961..59e962467a3d 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -716,11 +716,11 @@ static int axis_fifo_close(struct inode *inod, struct file *f)
>  }
>  
>  static const struct file_operations fops = {
> -	.owner = THIS_MODULE,
> -	.open = axis_fifo_open,
> +	.owner   = THIS_MODULE,
> +	.open    = axis_fifo_open,
>  	.release = axis_fifo_close,
> -	.read = axis_fifo_read,
> -	.write = axis_fifo_write
> +	.read    = axis_fifo_read,
> +	.write   = axis_fifo_write

Note this is only subjectively better. IMHO with just a single space
this is perfectly readable. Aligning the = might look nice, but it's
also annoying at times. When you add another member (e.g.
.iterate_shared) you either add a line that doesn't match all others, or
you have to touch all other lines of that struct which (objectively?)
hurts readability of that patch. Also for generated patches this kind of
alignment yields extra work. (See for example
https://lore.kernel.org/lkml/20230525205840.734432-1-u.kleine-koenig@pengutronix.de/
which required semi-manual fixup to keep the alignment after coccinelle
generated the patch.)

If you still think this is a good idea, I'd ask you to stick to one
style for the whole file. e.g. axis_fifo_driver uses inconsistent
and different indention.

A thing that IMHO is more useful to change here, is the name fops; I'd
suggest something like axis_fifo_fops (and also use prefixes for some
other symbols like "get_dts_property"). In 6.4-rc1 my ctags file knows
about 64 different places that define something called "fops".

Just my 0.02€,
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct
  2023-05-27 22:31               ` Uwe Kleine-König
@ 2023-05-28  8:57                 ` Greg Kroah-Hartman
  2023-06-13 16:01                   ` Prathu Baronia
  2023-06-13 15:56                 ` Prathu Baronia
  1 sibling, 1 reply; 25+ messages in thread
From: Greg Kroah-Hartman @ 2023-05-28  8:57 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Prathu Baronia, Fabio M. De Francesco, Khadija Kamran,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sun, May 28, 2023 at 12:31:11AM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> On Sat, May 27, 2023 at 05:21:00PM +0530, Prathu Baronia wrote:
> > Add required spaces for proper formatting of fops members for better
> > readability.
> > 
> > Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
> > ---
> >  drivers/staging/axis-fifo/axis-fifo.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> > index d71bdc6dd961..59e962467a3d 100644
> > --- a/drivers/staging/axis-fifo/axis-fifo.c
> > +++ b/drivers/staging/axis-fifo/axis-fifo.c
> > @@ -716,11 +716,11 @@ static int axis_fifo_close(struct inode *inod, struct file *f)
> >  }
> >  
> >  static const struct file_operations fops = {
> > -	.owner = THIS_MODULE,
> > -	.open = axis_fifo_open,
> > +	.owner   = THIS_MODULE,
> > +	.open    = axis_fifo_open,
> >  	.release = axis_fifo_close,
> > -	.read = axis_fifo_read,
> > -	.write = axis_fifo_write
> > +	.read    = axis_fifo_read,
> > +	.write   = axis_fifo_write
> 
> Note this is only subjectively better. IMHO with just a single space
> this is perfectly readable. Aligning the = might look nice, but it's
> also annoying at times. When you add another member (e.g.
> .iterate_shared) you either add a line that doesn't match all others, or
> you have to touch all other lines of that struct which (objectively?)
> hurts readability of that patch. Also for generated patches this kind of
> alignment yields extra work. (See for example
> https://lore.kernel.org/lkml/20230525205840.734432-1-u.kleine-koenig@pengutronix.de/
> which required semi-manual fixup to keep the alignment after coccinelle
> generated the patch.)
> 
> If you still think this is a good idea, I'd ask you to stick to one
> style for the whole file. e.g. axis_fifo_driver uses inconsistent
> and different indention.

I agree, there is no "requirement" that these fields are aligned at all,
so I would stick to the real fixes that are needed for this code to be
able to be moved out of staging instead.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct
  2023-05-28  8:57                 ` Greg Kroah-Hartman
@ 2023-06-13 16:01                   ` Prathu Baronia
  0 siblings, 0 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-13 16:01 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Uwe Kleine-König, Fabio M. De Francesco, Khadija Kamran,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sun, May 28, 2023 at 09:57:52AM +0100, Greg Kroah-Hartman wrote:
> I agree, there is no "requirement" that these fields are aligned at all,
> so I would stick to the real fixes that are needed for this code to be
> able to be moved out of staging instead.
Sure greg, leaving out the cleanup fixes for later.

Prathu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct
  2023-05-27 22:31               ` Uwe Kleine-König
  2023-05-28  8:57                 ` Greg Kroah-Hartman
@ 2023-06-13 15:56                 ` Prathu Baronia
  1 sibling, 0 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-13 15:56 UTC (permalink / raw)
  To: Uwe Kleine-König
  Cc: Greg Kroah-Hartman, Fabio M. De Francesco, Khadija Kamran,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sun, May 28, 2023 at 12:31:11AM +0200, Uwe Kleine-König wrote:
> Note this is only subjectively better. IMHO with just a single space
> this is perfectly readable. Aligning the = might look nice, but it's
> also annoying at times. When you add another member (e.g.
> .iterate_shared) you either add a line that doesn't match all others, or
> you have to touch all other lines of that struct which (objectively?)
> hurts readability of that patch. Also for generated patches this kind of
> alignment yields extra work. (See for example
> https://lore.kernel.org/lkml/20230525205840.734432-1-u.kleine-koenig@pengutronix.de/
> which required semi-manual fixup to keep the alignment after coccinelle
> generated the patch.)
> 
Agreed, that this would create more troubles than benefits.

> If you still think this is a good idea, I'd ask you to stick to one
> style for the whole file. e.g. axis_fifo_driver uses inconsistent
> and different indention.
> 
> A thing that IMHO is more useful to change here, is the name fops; I'd
> suggest something like axis_fifo_fops (and also use prefixes for some
> other symbols like "get_dts_property"). In 6.4-rc1 my ctags file knows
> about 64 different places that define something called "fops".
> 
Agreed. Upon further walkthrough I think this driver requires a lot of cleanup
so I will leave that cleanup for another dedicated patch series for now.

Prathu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier
  2023-05-27 11:50           ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Prathu Baronia
  2023-05-27 11:50             ` [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
  2023-05-27 11:51             ` [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct Prathu Baronia
@ 2023-05-28  8:59             ` Greg Kroah-Hartman
  2023-06-13 16:02               ` Prathu Baronia
  2 siblings, 1 reply; 25+ messages in thread
From: Greg Kroah-Hartman @ 2023-05-28  8:59 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: Uwe Kleine-König, Khadija Kamran, Fabio M. De Francesco,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sat, May 27, 2023 at 05:20:58PM +0530, Prathu Baronia wrote:
> Fix an old smatch warning reported by lkp introduced by commit
> d2d7aa53891e. In the mentioned commit we had used "%pa" format specifier
> for a void* type and hence smatch complained about its use instead of
> "%p".
> 
> Fixes: d2d7aa53891e ("staging: axis-fifo: convert to use miscdevice")
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <error27@gmail.com>
> Link: https://lore.kernel.org/r/202305150358.nt1BkbXz-lkp@intel.com/
> Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
> ---
> V3 -> V4: Split into warning fixing and cleanup commits
> V2 -> V3: Fix smatch warnings from kernel test robot
> V1 -> V2: Split into logical commits and fix commit message
> 
>  drivers/staging/axis-fifo/axis-fifo.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> index 7a21f2423204..271cab805cad 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -906,8 +906,8 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	if (rc < 0)
>  		goto err_initial;
>  
> -	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
> -		 &r_mem->start, &fifo->base_addr, fifo->irq);
> +	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%p, irq=%i\n",
> +		 &r_mem->start, fifo->base_addr, fifo->irq);

In looking at this further, this whole dev_info() line should just be
removed.  When drivers work properly, they are quiet, otherwise the
kernel log would look even worse than it currently is.

So please just remove this entirely.  Also, attempting to print memory
addresses to the kernel log is very suspect and generally frowned apon,
which is another reason this shouldn't be done.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier
  2023-05-28  8:59             ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Greg Kroah-Hartman
@ 2023-06-13 16:02               ` Prathu Baronia
  2023-06-16 15:25                 ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
  0 siblings, 1 reply; 25+ messages in thread
From: Prathu Baronia @ 2023-06-13 16:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Uwe Kleine-König, Khadija Kamran, Fabio M. De Francesco,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sun, May 28, 2023 at 09:59:27AM +0100, Greg Kroah-Hartman wrote:
> So please just remove this entirely.  Also, attempting to print memory
> addresses to the kernel log is very suspect and generally frowned apon,
> which is another reason this shouldn't be done.
Agreed, will be removed in v5.

Prathu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-06-13 16:02               ` Prathu Baronia
@ 2023-06-16 15:25                 ` Prathu Baronia
  2023-06-16 15:26                   ` [PATCH v5 2/2] axis-fifo: remove the unnecessary dev_info() Prathu Baronia
  2023-06-17 14:00                   ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg Kroah-Hartman
  0 siblings, 2 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-16 15:25 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Fabio M. De Francesco,
	Khadija Kamran, Uwe Kleine-König, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

In various places, string buffers of a fixed size are allocated, and
filled using snprintf() with the same fixed size, which is error-prone.

Replace this by calling devm_kasprintf() instead, which always uses the
appropriate size. Allocate the device name with a unique identifier
instead of a kernel address.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
V4 -> V5: Remove the dev_info() and use a unique identifier for dev name
V3 -> V4: Split into warning fixing and cleanup commits
V2 -> V3: Fix smatch warnings from kernel test robot
V1 -> V2: Split into logical commits and fix commit message

 drivers/staging/axis-fifo/axis-fifo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 7a21f2423204..5e070e00e27a 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -816,10 +816,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	 * ----------------------------
 	 */
 
-	device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!device_name)
-		return -ENOMEM;
-
 	/* allocate device wrapper memory */
 	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
 	if (!fifo)
@@ -857,7 +853,11 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
 
 	/* create unique device name */
-	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
+	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%d", DRIVER_NAME, pdev->id);
+	if (!device_name) {
+		rc = -ENOMEM;
+		goto err_initial;
+	}
 	dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
 
 	/* ----------------------------

base-commit: fb054096aea0576f0c0a61c598e5e9676443ee86
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v5 2/2] axis-fifo: remove the unnecessary dev_info()
  2023-06-16 15:25                 ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
@ 2023-06-16 15:26                   ` Prathu Baronia
  2023-06-17 14:00                   ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg Kroah-Hartman
  1 sibling, 0 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-16 15:26 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Fabio M. De Francesco,
	Khadija Kamran, Uwe Kleine-König, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

This dev_info() statement is not needed since drivers need to be quiet
under normal operation and its not a good idea to print addresses in
kernel log.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 5e070e00e27a..d1ce8b9e32eb 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -906,9 +906,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	if (rc < 0)
 		goto err_initial;
 
-	dev_info(fifo->dt_device, "axis-fifo created at %pa mapped to 0x%pa, irq=%i\n",
-		 &r_mem->start, &fifo->base_addr, fifo->irq);
-
 	return 0;
 
 err_initial:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-06-16 15:25                 ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
  2023-06-16 15:26                   ` [PATCH v5 2/2] axis-fifo: remove the unnecessary dev_info() Prathu Baronia
@ 2023-06-17 14:00                   ` Greg Kroah-Hartman
  2023-06-19  6:43                     ` Prathu Baronia
  1 sibling, 1 reply; 25+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-17 14:00 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: Fabio M. De Francesco, Khadija Kamran, Uwe Kleine-König,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Fri, Jun 16, 2023 at 08:55:59PM +0530, Prathu Baronia wrote:
> In various places, string buffers of a fixed size are allocated, and
> filled using snprintf() with the same fixed size, which is error-prone.
> 
> Replace this by calling devm_kasprintf() instead, which always uses the
> appropriate size. Allocate the device name with a unique identifier
> instead of a kernel address.

You are doing two different things here, one is changing the allocation
way, and the other is the name.  If one of those things turns out to
break something, we have to revert this whole thing.

So please make this two different changes, one to change to use
devm_kasprintf() and the second to change the naming scheme, ESPECIALLY
as you do not mention the name change in the subject line.  And that's
going to be a user-visible change, so you need to make that VERY
obvious.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-06-17 14:00                   ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg Kroah-Hartman
@ 2023-06-19  6:43                     ` Prathu Baronia
  2023-06-19 16:22                       ` [PATCH v6 " Prathu Baronia
  0 siblings, 1 reply; 25+ messages in thread
From: Prathu Baronia @ 2023-06-19  6:43 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Fabio M. De Francesco, Khadija Kamran, Uwe Kleine-König,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Sat, Jun 17, 2023 at 04:00:02PM +0200, Greg Kroah-Hartman wrote:
> 
> You are doing two different things here, one is changing the allocation
> way, and the other is the name.  If one of those things turns out to
> break something, we have to revert this whole thing.
> 
> So please make this two different changes, one to change to use
> devm_kasprintf() and the second to change the naming scheme, ESPECIALLY
> as you do not mention the name change in the subject line.  And that's
> going to be a user-visible change, so you need to make that VERY
> obvious.
Makes sense greg, will do in v6.

Prathu

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v6 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-06-19  6:43                     ` Prathu Baronia
@ 2023-06-19 16:22                       ` Prathu Baronia
  2023-06-19 16:22                         ` [PATCH v6 2/2] axis-fifo: change device name by assigning unique device id Prathu Baronia
                                           ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-19 16:22 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Fabio M. De Francesco,
	Khadija Kamran, Uwe Kleine-König, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

In various places, string buffers of a fixed size are allocated, and
filled using snprintf() with the same fixed size, which is error-prone.

Replace this by calling devm_kasprintf() instead, which always uses the
appropriate size.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
V5 -> V6: Split into api change and name change commits
V4 -> V5: Remove the dev_info() and use a unique identifier for dev name
V3 -> V4: Split into warning fixing and cleanup commits
V2 -> V3: Fix smatch warnings from kernel test robot
V1 -> V2: Split into logical commits and fix commit message

 drivers/staging/axis-fifo/axis-fifo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 7a21f2423204..7d8da9ce2db8 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -816,10 +816,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	 * ----------------------------
 	 */
 
-	device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
-	if (!device_name)
-		return -ENOMEM;
-
 	/* allocate device wrapper memory */
 	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
 	if (!fifo)
@@ -857,7 +853,11 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
 
 	/* create unique device name */
-	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
+	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%p", DRIVER_NAME, &r_mem->start);
+	if (!device_name) {
+		rc = -ENOMEM;
+		goto err_initial;
+	}
 	dev_dbg(fifo->dt_device, "device name [%s]\n", device_name);
 
 	/* ----------------------------

base-commit: 45a3e24f65e90a047bef86f927ebdc4c710edaa1
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH v6 2/2] axis-fifo: change device name by assigning unique device id
  2023-06-19 16:22                       ` [PATCH v6 " Prathu Baronia
@ 2023-06-19 16:22                         ` Prathu Baronia
  2023-06-20  5:18                         ` [PATCH v6 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Dan Carpenter
  2023-06-20  9:24                         ` Greg Kroah-Hartman
  2 siblings, 0 replies; 25+ messages in thread
From: Prathu Baronia @ 2023-06-19 16:22 UTC (permalink / raw)
  To: prathubaronia2011, Greg Kroah-Hartman, Uwe Kleine-König,
	Fabio M. De Francesco, Khadija Kamran, linux-staging,
	linux-kernel
  Cc: dan.carpenter, error27, lkp, oe-kbuild-all, oe-kbuild

Allocating the device name with a unique identifier
instead of a kernel address.

Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
---
 drivers/staging/axis-fifo/axis-fifo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
index 7d8da9ce2db8..5e070e00e27a 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -853,7 +853,7 @@ static int axis_fifo_probe(struct platform_device *pdev)
 	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
 
 	/* create unique device name */
-	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%p", DRIVER_NAME, &r_mem->start);
+	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%d", DRIVER_NAME, pdev->id);
 	if (!device_name) {
 		rc = -ENOMEM;
 		goto err_initial;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH v6 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-06-19 16:22                       ` [PATCH v6 " Prathu Baronia
  2023-06-19 16:22                         ` [PATCH v6 2/2] axis-fifo: change device name by assigning unique device id Prathu Baronia
@ 2023-06-20  5:18                         ` Dan Carpenter
  2023-06-20  9:24                         ` Greg Kroah-Hartman
  2 siblings, 0 replies; 25+ messages in thread
From: Dan Carpenter @ 2023-06-20  5:18 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: Greg Kroah-Hartman, Fabio M. De Francesco, Khadija Kamran,
	Uwe Kleine-König, linux-staging, linux-kernel, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Mon, Jun 19, 2023 at 09:52:44PM +0530, Prathu Baronia wrote:
> In various places, string buffers of a fixed size are allocated, and
> filled using snprintf() with the same fixed size, which is error-prone.
> 
> Replace this by calling devm_kasprintf() instead, which always uses the
> appropriate size.
> 
> Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
> ---
> V5 -> V6: Split into api change and name change commits
> V4 -> V5: Remove the dev_info() and use a unique identifier for dev name
> V3 -> V4: Split into warning fixing and cleanup commits
> V2 -> V3: Fix smatch warnings from kernel test robot
> V1 -> V2: Split into logical commits and fix commit message
> 

> -	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
                                      ^^^

> +	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%p", DRIVER_NAME, &r_mem->start);
                                                          ^^

This is a sneaky fix which Greg already kind of complained about if I
remember correctly...

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v6 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings
  2023-06-19 16:22                       ` [PATCH v6 " Prathu Baronia
  2023-06-19 16:22                         ` [PATCH v6 2/2] axis-fifo: change device name by assigning unique device id Prathu Baronia
  2023-06-20  5:18                         ` [PATCH v6 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Dan Carpenter
@ 2023-06-20  9:24                         ` Greg Kroah-Hartman
  2 siblings, 0 replies; 25+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-20  9:24 UTC (permalink / raw)
  To: Prathu Baronia
  Cc: Fabio M. De Francesco, Khadija Kamran, Uwe Kleine-König,
	linux-staging, linux-kernel, dan.carpenter, error27, lkp,
	oe-kbuild-all, oe-kbuild

On Mon, Jun 19, 2023 at 09:52:44PM +0530, Prathu Baronia wrote:
> In various places, string buffers of a fixed size are allocated, and
> filled using snprintf() with the same fixed size, which is error-prone.
> 
> Replace this by calling devm_kasprintf() instead, which always uses the
> appropriate size.
> 
> Signed-off-by: Prathu Baronia <prathubaronia2011@gmail.com>
> ---
> V5 -> V6: Split into api change and name change commits
> V4 -> V5: Remove the dev_info() and use a unique identifier for dev name
> V3 -> V4: Split into warning fixing and cleanup commits
> V2 -> V3: Fix smatch warnings from kernel test robot
> V1 -> V2: Split into logical commits and fix commit message
> 
>  drivers/staging/axis-fifo/axis-fifo.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/axis-fifo/axis-fifo.c b/drivers/staging/axis-fifo/axis-fifo.c
> index 7a21f2423204..7d8da9ce2db8 100644
> --- a/drivers/staging/axis-fifo/axis-fifo.c
> +++ b/drivers/staging/axis-fifo/axis-fifo.c
> @@ -816,10 +816,6 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	 * ----------------------------
>  	 */
>  
> -	device_name = devm_kzalloc(dev, 32, GFP_KERNEL);
> -	if (!device_name)
> -		return -ENOMEM;
> -
>  	/* allocate device wrapper memory */
>  	fifo = devm_kzalloc(dev, sizeof(*fifo), GFP_KERNEL);
>  	if (!fifo)
> @@ -857,7 +853,11 @@ static int axis_fifo_probe(struct platform_device *pdev)
>  	dev_dbg(fifo->dt_device, "remapped memory to 0x%p\n", fifo->base_addr);
>  
>  	/* create unique device name */
> -	snprintf(device_name, 32, "%s_%pa", DRIVER_NAME, &r_mem->start);
> +	device_name = devm_kasprintf(dev, GFP_KERNEL, "%s_%p", DRIVER_NAME, &r_mem->start);

As Dan points out, you are changing the name here :(


^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2023-06-20  9:24 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20230514130148.138624-2-prathubaronia2011@gmail.com>
2023-05-15  7:42 ` [PATCH v2 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Dan Carpenter
2023-05-18 14:31   ` Prathu Baronia
2023-05-18 14:51     ` [PATCH v3 " Prathu Baronia
2023-05-18 14:51       ` [PATCH v3 2/2] axis-fifo: cleanup space issues with fops struct Prathu Baronia
2023-05-27  7:35       ` [PATCH v3 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg KH
2023-05-27  8:53         ` Prathu Baronia
2023-05-27 11:50           ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Prathu Baronia
2023-05-27 11:50             ` [PATCH v4 2/3] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
2023-05-28  9:00               ` Greg Kroah-Hartman
2023-06-13 16:03                 ` Prathu Baronia
2023-05-27 11:51             ` [PATCH v4 3/3] axis-fifo: cleanup space issues with fops struct Prathu Baronia
2023-05-27 22:31               ` Uwe Kleine-König
2023-05-28  8:57                 ` Greg Kroah-Hartman
2023-06-13 16:01                   ` Prathu Baronia
2023-06-13 15:56                 ` Prathu Baronia
2023-05-28  8:59             ` [PATCH v4 1/3] axis-fifo: fix smatch warning for format specifier Greg Kroah-Hartman
2023-06-13 16:02               ` Prathu Baronia
2023-06-16 15:25                 ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Prathu Baronia
2023-06-16 15:26                   ` [PATCH v5 2/2] axis-fifo: remove the unnecessary dev_info() Prathu Baronia
2023-06-17 14:00                   ` [PATCH v5 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Greg Kroah-Hartman
2023-06-19  6:43                     ` Prathu Baronia
2023-06-19 16:22                       ` [PATCH v6 " Prathu Baronia
2023-06-19 16:22                         ` [PATCH v6 2/2] axis-fifo: change device name by assigning unique device id Prathu Baronia
2023-06-20  5:18                         ` [PATCH v6 1/2] axis-fifo: use devm_kasprintf() for allocating formatted strings Dan Carpenter
2023-06-20  9:24                         ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox