Coccinelle Archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] cocci: missed strlcpy->strscpy conversion?
       [not found] <58a84d03b714f71d231f9cac04af09a6b97c6f04.camel@perches.com>
@ 2020-12-31 20:13 ` Joe Perches
  2020-12-31 20:27   ` Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2020-12-31 20:13 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci

On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote:
> strlcpy is deprecated.  see: Documentation/process/deprecated.rst
> 
> Change the calls that do not use the strlcpy return value to the
> preferred strscpy.
> 
> Done with cocci script:
> 
> @@
> expression e1, e2, e3;
> @@
> 
> -	strlcpy(
> +	strscpy(
> 	e1, e2, e3);
> 
> This cocci script leaves the instances where the return value is
> used unchanged.

Hey Julia.

After using the cocci script above on a test treewide conversion,
there were a few instances with no return use that were not converted.

Any idea why these were not converted?
I don't see a pattern.

The .h files may be because those are the only uses in .h files in the kernel
but drivers/block/rnbd/rnbd-clt.c I don't understand at all.

drivers/block/rnbd/rnbd-clt.c:  strlcpy(sess->sessname, sessname, sizeof(sess->sessname));
drivers/input/serio/i8042-x86ia64io.h:  strlcpy(dst, "PNP:", dst_size);
drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_kbd_name, did->id, sizeof(i8042_pnp_kbd_name));
drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_aux_name, did->id, sizeof(i8042_pnp_aux_name));
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h:      strlcpy(buf, bp->acquire_resp.pfdev_info.fw_ver, buf_len);

$ git grep -3 strlcpy drivers/block/rnbd/rnbd-clt.c drivers/input/serio/i8042-x86ia64io.h drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
drivers/block/rnbd/rnbd-clt.c-  sess = kzalloc_node(sizeof(*sess), GFP_KERNEL, NUMA_NO_NODE);
drivers/block/rnbd/rnbd-clt.c-  if (!sess)
drivers/block/rnbd/rnbd-clt.c-          return ERR_PTR(-ENOMEM);
drivers/block/rnbd/rnbd-clt.c:  strlcpy(sess->sessname, sessname, sizeof(sess->sessname));
drivers/block/rnbd/rnbd-clt.c-  atomic_set(&sess->busy, 0);
drivers/block/rnbd/rnbd-clt.c-  mutex_init(&sess->lock);
drivers/block/rnbd/rnbd-clt.c-  INIT_LIST_HEAD(&sess->devs_list);
--
drivers/input/serio/i8042-x86ia64io.h-
drivers/input/serio/i8042-x86ia64io.h-static void i8042_pnp_id_to_string(struct pnp_id *id, char *dst, int dst_size)
drivers/input/serio/i8042-x86ia64io.h-{
drivers/input/serio/i8042-x86ia64io.h:  strlcpy(dst, "PNP:", dst_size);
drivers/input/serio/i8042-x86ia64io.h-
drivers/input/serio/i8042-x86ia64io.h-  while (id) {
drivers/input/serio/i8042-x86ia64io.h-          strlcat(dst, " ", dst_size);
--
drivers/input/serio/i8042-x86ia64io.h-  if (pnp_irq_valid(dev,0))
drivers/input/serio/i8042-x86ia64io.h-          i8042_pnp_kbd_irq = pnp_irq(dev, 0);
drivers/input/serio/i8042-x86ia64io.h-
drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_kbd_name, did->id, sizeof(i8042_pnp_kbd_name));
drivers/input/serio/i8042-x86ia64io.h-  if (strlen(pnp_dev_name(dev))) {
drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_kbd_name, ":", sizeof(i8042_pnp_kbd_name));
drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_kbd_name, pnp_dev_name(dev), sizeof(i8042_pnp_kbd_name));
--
drivers/input/serio/i8042-x86ia64io.h-  if (pnp_irq_valid(dev, 0))
drivers/input/serio/i8042-x86ia64io.h-          i8042_pnp_aux_irq = pnp_irq(dev, 0);
drivers/input/serio/i8042-x86ia64io.h-
drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_aux_name, did->id, sizeof(i8042_pnp_aux_name));
drivers/input/serio/i8042-x86ia64io.h-  if (strlen(pnp_dev_name(dev))) {
drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_aux_name, ":", sizeof(i8042_pnp_aux_name));
drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_aux_name, pnp_dev_name(dev), sizeof(i8042_pnp_aux_name));
--
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-static inline void bnx2x_vf_fill_fw_str(struct bnx2x *bp, char *buf,
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-                                      size_t buf_len)
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-{
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h:      strlcpy(buf, bp->acquire_resp.pfdev_info.fw_ver, buf_len);
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-}
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-static inline int bnx2x_vf_ustorm_prods_offset(struct bnx2x *bp,

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
  2020-12-31 20:13 ` [Cocci] cocci: missed strlcpy->strscpy conversion? Joe Perches
@ 2020-12-31 20:27   ` Julia Lawall
  2020-12-31 20:41     ` Joe Perches
  0 siblings, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2020-12-31 20:27 UTC (permalink / raw)
  To: Joe Perches; +Cc: cocci



On Thu, 31 Dec 2020, Joe Perches wrote:

> On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote:
> > strlcpy is deprecated.  see: Documentation/process/deprecated.rst
> >
> > Change the calls that do not use the strlcpy return value to the
> > preferred strscpy.
> >
> > Done with cocci script:
> >
> > @@
> > expression e1, e2, e3;
> > @@
> >
> > -	strlcpy(
> > +	strscpy(
> > 	e1, e2, e3);
> >
> > This cocci script leaves the instances where the return value is
> > used unchanged.
>
> Hey Julia.
>
> After using the cocci script above on a test treewide conversion,
> there were a few instances with no return use that were not converted.
>
> Any idea why these were not converted?
> I don't see a pattern.

For a semantic patch like this, where you don't case about type
information and the change is very local, you can use the options:

--no-includes --include-headers

Then the .c files and the .h files will be treated one by one.  The
--no-includes options prevents the .h files from being included into the
.c files, which could causetheir code to get transformed at each
inclusion.  The --include-headers option causes the .h files to be
considered.

>
> The .h files may be because those are the only uses in .h files in the kernel
> but drivers/block/rnbd/rnbd-clt.c I don't understand at all.

The parse is not happy about the for_each_possible_cpu.  It seems that the
heuristic for detecting that as a loop expects that the body of the loop
will have braces.  You can see this with the --parse-c option, ie

spatch --parse-c drivers/block/rnbd/rnbd-clt.c

The offending line will have BAD in front of it.

julia

> drivers/block/rnbd/rnbd-clt.c:  strlcpy(sess->sessname, sessname, sizeof(sess->sessname));
> drivers/input/serio/i8042-x86ia64io.h:  strlcpy(dst, "PNP:", dst_size);
> drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_kbd_name, did->id, sizeof(i8042_pnp_kbd_name));
> drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_aux_name, did->id, sizeof(i8042_pnp_aux_name));
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h:      strlcpy(buf, bp->acquire_resp.pfdev_info.fw_ver, buf_len);
>
> $ git grep -3 strlcpy drivers/block/rnbd/rnbd-clt.c drivers/input/serio/i8042-x86ia64io.h drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h
> drivers/block/rnbd/rnbd-clt.c-  sess = kzalloc_node(sizeof(*sess), GFP_KERNEL, NUMA_NO_NODE);
> drivers/block/rnbd/rnbd-clt.c-  if (!sess)
> drivers/block/rnbd/rnbd-clt.c-          return ERR_PTR(-ENOMEM);
> drivers/block/rnbd/rnbd-clt.c:  strlcpy(sess->sessname, sessname, sizeof(sess->sessname));
> drivers/block/rnbd/rnbd-clt.c-  atomic_set(&sess->busy, 0);
> drivers/block/rnbd/rnbd-clt.c-  mutex_init(&sess->lock);
> drivers/block/rnbd/rnbd-clt.c-  INIT_LIST_HEAD(&sess->devs_list);
> --
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h-static void i8042_pnp_id_to_string(struct pnp_id *id, char *dst, int dst_size)
> drivers/input/serio/i8042-x86ia64io.h-{
> drivers/input/serio/i8042-x86ia64io.h:  strlcpy(dst, "PNP:", dst_size);
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h-  while (id) {
> drivers/input/serio/i8042-x86ia64io.h-          strlcat(dst, " ", dst_size);
> --
> drivers/input/serio/i8042-x86ia64io.h-  if (pnp_irq_valid(dev,0))
> drivers/input/serio/i8042-x86ia64io.h-          i8042_pnp_kbd_irq = pnp_irq(dev, 0);
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_kbd_name, did->id, sizeof(i8042_pnp_kbd_name));
> drivers/input/serio/i8042-x86ia64io.h-  if (strlen(pnp_dev_name(dev))) {
> drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_kbd_name, ":", sizeof(i8042_pnp_kbd_name));
> drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_kbd_name, pnp_dev_name(dev), sizeof(i8042_pnp_kbd_name));
> --
> drivers/input/serio/i8042-x86ia64io.h-  if (pnp_irq_valid(dev, 0))
> drivers/input/serio/i8042-x86ia64io.h-          i8042_pnp_aux_irq = pnp_irq(dev, 0);
> drivers/input/serio/i8042-x86ia64io.h-
> drivers/input/serio/i8042-x86ia64io.h:  strlcpy(i8042_pnp_aux_name, did->id, sizeof(i8042_pnp_aux_name));
> drivers/input/serio/i8042-x86ia64io.h-  if (strlen(pnp_dev_name(dev))) {
> drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_aux_name, ":", sizeof(i8042_pnp_aux_name));
> drivers/input/serio/i8042-x86ia64io.h-          strlcat(i8042_pnp_aux_name, pnp_dev_name(dev), sizeof(i8042_pnp_aux_name));
> --
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-static inline void bnx2x_vf_fill_fw_str(struct bnx2x *bp, char *buf,
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-                                      size_t buf_len)
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-{
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h:      strlcpy(buf, bp->acquire_resp.pfdev_info.fw_ver, buf_len);
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-}
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h-static inline int bnx2x_vf_ustorm_prods_offset(struct bnx2x *bp,
>
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
  2020-12-31 20:27   ` Julia Lawall
@ 2020-12-31 20:41     ` Joe Perches
  2020-12-31 20:49       ` Julia Lawall
  2020-12-31 21:21       ` Julia Lawall
  0 siblings, 2 replies; 6+ messages in thread
From: Joe Perches @ 2020-12-31 20:41 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci

On Thu, 2020-12-31 at 21:27 +0100, Julia Lawall wrote:
> On Thu, 31 Dec 2020, Joe Perches wrote:
> > On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote:
> > > strlcpy is deprecated.  see: Documentation/process/deprecated.rst
> > > 
> > > Change the calls that do not use the strlcpy return value to the
> > > preferred strscpy.
> > > 
> > > Done with cocci script:
> > > 
> > > @@
> > > expression e1, e2, e3;
> > > @@
> > > 
> > > -	strlcpy(
> > > +	strscpy(
> > > 	e1, e2, e3);
> > > 
> > > This cocci script leaves the instances where the return value is
> > > used unchanged.
> > 
> > Hey Julia.
> > 
> > After using the cocci script above on a test treewide conversion,
> > there were a few instances with no return use that were not converted.
> > 
> > Any idea why these were not converted?
[]
> The parse is not happy about the for_each_possible_cpu.  It seems that the
> heuristic for detecting that as a loop expects that the body of the loop
> will have braces.  You can see this with the --parse-c option, ie
> 
> spatch --parse-c drivers/block/rnbd/rnbd-clt.c
> 
> The offending line will have BAD in front of it.

Thanks.

Do you consider the for_each heuristic a defect? (I'm not sure I do)

I hope it's time for you to stop working today...

cheers and happy new year,  Joe

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
  2020-12-31 20:41     ` Joe Perches
@ 2020-12-31 20:49       ` Julia Lawall
  2020-12-31 21:21       ` Julia Lawall
  1 sibling, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2020-12-31 20:49 UTC (permalink / raw)
  To: Joe Perches; +Cc: cocci



On Thu, 31 Dec 2020, Joe Perches wrote:

> On Thu, 2020-12-31 at 21:27 +0100, Julia Lawall wrote:
> > On Thu, 31 Dec 2020, Joe Perches wrote:
> > > On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote:
> > > > strlcpy is deprecated.  see: Documentation/process/deprecated.rst
> > > >
> > > > Change the calls that do not use the strlcpy return value to the
> > > > preferred strscpy.
> > > >
> > > > Done with cocci script:
> > > >
> > > > @@
> > > > expression e1, e2, e3;
> > > > @@
> > > >
> > > > -	strlcpy(
> > > > +	strscpy(
> > > > 	e1, e2, e3);
> > > >
> > > > This cocci script leaves the instances where the return value is
> > > > used unchanged.
> > >
> > > Hey Julia.
> > >
> > > After using the cocci script above on a test treewide conversion,
> > > there were a few instances with no return use that were not converted.
> > >
> > > Any idea why these were not converted?
> []
> > The parse is not happy about the for_each_possible_cpu.  It seems that the
> > heuristic for detecting that as a loop expects that the body of the loop
> > will have braces.  You can see this with the --parse-c option, ie
> >
> > spatch --parse-c drivers/block/rnbd/rnbd-clt.c
> >
> > The offending line will have BAD in front of it.
>
> Thanks.
>
> Do you consider the for_each heuristic a defect? (I'm not sure I do)

It could be improved.  I was wondering if the indentation was not correct.
If there was a space in the line with the for_each, then the two lines
would be equally indented, which would certainly confuse the heuristics.
But that is not the case; the indentation is fine.

And the file contains a previous for_each_possible_cpu that has braces, so
it should have taken note of that and detected that the next one should
also be a loop.

>
> I hope it's time for you to stop working today...
>
> cheers and happy new year,  Joe

Thanks!  Happy new year :)

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
  2020-12-31 20:41     ` Joe Perches
  2020-12-31 20:49       ` Julia Lawall
@ 2020-12-31 21:21       ` Julia Lawall
  2020-12-31 21:23         ` Joe Perches
  1 sibling, 1 reply; 6+ messages in thread
From: Julia Lawall @ 2020-12-31 21:21 UTC (permalink / raw)
  To: Joe Perches; +Cc: cocci



On Thu, 31 Dec 2020, Joe Perches wrote:

> On Thu, 2020-12-31 at 21:27 +0100, Julia Lawall wrote:
> > On Thu, 31 Dec 2020, Joe Perches wrote:
> > > On Thu, 2020-12-31 at 11:04 -0800, Joe Perches wrote:
> > > > strlcpy is deprecated.  see: Documentation/process/deprecated.rst
> > > >
> > > > Change the calls that do not use the strlcpy return value to the
> > > > preferred strscpy.
> > > >
> > > > Done with cocci script:
> > > >
> > > > @@
> > > > expression e1, e2, e3;
> > > > @@
> > > >
> > > > -	strlcpy(
> > > > +	strscpy(
> > > > 	e1, e2, e3);
> > > >
> > > > This cocci script leaves the instances where the return value is
> > > > used unchanged.
> > >
> > > Hey Julia.
> > >
> > > After using the cocci script above on a test treewide conversion,
> > > there were a few instances with no return use that were not converted.
> > >
> > > Any idea why these were not converted?
> []
> > The parse is not happy about the for_each_possible_cpu.  It seems that the
> > heuristic for detecting that as a loop expects that the body of the loop
> > will have braces.  You can see this with the --parse-c option, ie
> >
> > spatch --parse-c drivers/block/rnbd/rnbd-clt.c
> >
> > The offending line will have BAD in front of it.
>
> Thanks.
>
> Do you consider the for_each heuristic a defect? (I'm not sure I do)

It seems that the problem is not really the for_each, but the * in front
of a "function call" on the left side of an assignment.  Without the *,
everything is fine.  So it is indeed probably not worth doing anything
about.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

* Re: [Cocci] cocci: missed strlcpy->strscpy conversion?
  2020-12-31 21:21       ` Julia Lawall
@ 2020-12-31 21:23         ` Joe Perches
  0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2020-12-31 21:23 UTC (permalink / raw)
  To: Julia Lawall; +Cc: cocci

On Thu, 2020-12-31 at 22:21 +0100, Julia Lawall wrote:
> It seems that the problem is not really the for_each, but the * in front
> of a "function call" on the left side of an assignment.  Without the *,
> everything is fine.  So it is indeed probably not worth doing anything
> about.

Maybe a combination because if braces are added to the for_each,
the conversion also works.


_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <58a84d03b714f71d231f9cac04af09a6b97c6f04.camel@perches.com>
2020-12-31 20:13 ` [Cocci] cocci: missed strlcpy->strscpy conversion? Joe Perches
2020-12-31 20:27   ` Julia Lawall
2020-12-31 20:41     ` Joe Perches
2020-12-31 20:49       ` Julia Lawall
2020-12-31 21:21       ` Julia Lawall
2020-12-31 21:23         ` Joe Perches

Coccinelle Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr
	public-inbox-index cocci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git