Re: [Cocci] Replacing a struct member with a function call

[Wolfram Sang]

> @@
> type M;

This?

struct monitor* m;

> @@
> - m->virtual.width;
> + get_monitor_width();


signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Supporting Coccinelle software

[Wolfram Sang]

On Fri, Feb 14, 2020 at 01:23:16PM +0100, Markus Elfring wrote:
> > I cloned coccinelle's GIT repository, I suggested a patch,
> > and the patch has been accepted.
> 
> Will the chances increase for further contributions?

Why do you want to know?



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()

[Wolfram Sang]

> these drivers pop up, I think we can have another function like
> platform_get_irq_probe() or platform_get_irq_nowarn() that doesn't print
> an error message. Then we can convert the drivers that are poking around
> for interrupts to use this new function instead. It isn't the same as a
> platform_get_optional_irq() API because it returns an error when the irq
> isn't there or we fail to parse something, but at least the error
> message is gone.

True.

I still feel uneasy about pushing false positive error messages to
users. Do you think your cocci-script could be updated to modify drivers
which do not bail out when platform_get_irq() fails to use
platform_get_irq_nowarn()? I'd think this would catch most of them?

Or maybe the other way around? platform_get_irq_warn() and only convert
those which print something?



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH v5 0/3] Add error message to platform_get_irq*()

[Wolfram Sang]

Hi Stephen,

> There were some comments about adding an 'optional' platform_get_irq()
> API in v4. This series doesn't include that, but I can add such an API
> if it's required. I started to look into how it might work and got hung
> up on what an optional IRQ means. I suppose it means that in DT there
> isn't an 'interrupts' property in the device node, but in ACPI based
> firmware I'm not sure what that would correspond to. Furthermore, the
> return value is hard to comprehend. Do we return an error when an
> optional irq can't be found? It doesn't seem safe to return 0 because
> sometimes 0 is a valid IRQ. Do other errors in parsing the IRQ
> constitute a failure when the IRQ is optional?

Some time ago, I tried a series like yours and got stuck at this very
point. I found drivers where using an interrupt was optional and
platform_get_irq() returning a failure wasn't fatal. The drivers used
PIO then or dropped some additional functionality. Some of them were
very old.

I didn't like the idea that platform_get_irq() will spit out errors for
those drivers, yet I couldn't create a suitable cocci-script to convert
drivers to use the *_optional callback where possible. So, I neither
created the optional callback.

I still have doubts of unneeded error messages popping up. Has this been
discussed already? (Sorry, I missed the first iterations of this series)

Thanks,

   Wolfram



signature.asc
Description: PGP signature

Re: [Cocci] [v5] coccinelle: semantic code search for missingput_device()

[Wolfram Sang]

Hi,

On Sat, Feb 16, 2019 at 09:57:54AM +0100, Markus Elfring wrote:
> > We don't need perfection.
> 
> I guess that you noticed in the meantime that I dare to propose
> more software development efforts in such a direction.

Yes, this is noticable. It is your choice, however, other people may
have their reasons for other choices...

> > We need more to eliminate the memory leaks.

... like this one.

> Will this view evolve into further helpful and constructive clarifications?

Given my above, what is the evaluation of the same question to yourself?

Regards,

   Wolfram



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] award for Coccinelle paper

[Wolfram Sang]

Julia,

> > received the Eurosys "Test of Time" award.  Many thanks to everyone on
> > this list who has helped to make this possible.

Congratulations from me, too! Very well deserved!

> Warming people up, ready for the major award for Coccinelle

What would that be?

Best wishes,

   Wolfram



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] Coccinelle: Script to replace NULL test with IS_ERR test for devm_ioremap_resource

[Wolfram Sang]

> I am working on a more general solution, extending to other functions.

Sounds good, thanks!



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] Coccinelle: Script to replace NULL test with IS_ERR test for devm_ioremap_resource

[Wolfram Sang]

On Thu, Jun 30, 2016 at 12:03:47AM +0530, Amitoj Kaur Chawla wrote:
> This script detects cases which have incorrect error handling for
> devm_ioremap_resource function, employing a NULL test instead of an
> IS_ERR() test.
> 
> Signed-off-by: Amitoj Kaur Chawla 

Why don't we fix the code at the same time?

And it should not be restricted to devm_ioremap_resource() but
extensible so other functions could be added later?

(Surprised to see that we don't have such a script yet)



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Wolfram Sang]

> Is there another scripts/coccinelle/ file I can use to test against to demo
> against glimpse/idutils/gitgrep best?

I'd think this one may be a candidate:

scripts/coccinelle/misc/irqf_oneshot.cocci

Not too many, but quite some matches over the tree.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Wolfram Sang]

> works pretty well, and there are quite a lot of files (7514) that contain 
> kfree.

Ah, kfree. That explains, I missed that info.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Wolfram Sang]

> It's not as efficient as glimpse because the query language is simpler.  

Interesting, what is missing compared to glimpse?

> So more filtering has to be done at the ocaml level.  But it's probably 
> fine in most cases.

For me, it has two advantages over glimpse:

a) it is in the debian package repository
b) the same database can be used with the code browser 'seascope'
   which can do nice things by feeding ctags on the fly with data
   from id-utils.

Mileages vary, of course, just wanted to mention it to give pointers.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Wolfram Sang]

> real16m11.692s
> user127m50.388s
> sys 0m2.168s

That's better but not a magnitude, I wonder.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Wolfram Sang]

> AFAICT coccinelle does not have integration support for id-utils though.

I used it just today ;) -- "--use-idutils ./ID"

ID was generated with simple 'mkid -s'.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 4/4] coccicheck: add indexing enhancement options

[Wolfram Sang]

> > Well, slightly better.
> 
> No, it should be much better.  You would have to look at the standard 

I use id-utils regularly and it is indeed at least a magnitude better.
The indexing often pays off already with the first coccinelle run for
me. Highly recommended.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] Coccinelle: setup_timer: Add space in front of parentheses

[Wolfram Sang]

On Sun, Mar 20, 2016 at 10:57:32AM +0530, Vaishali Thakkar wrote:
> Add space in front of the offending parentheses to silent the
> parse error for older Coccinelle versions. This makes the rule
> usable with all Coccinelle versions.
> 
> Reported-by: Nishanth Menon <n...@ti.com>
> Signed-off-by: Vaishali Thakkar <vaishali.thak...@oracle.com>

Tested-by: Wolfram Sang <w...@the-dreams.de>

Can we have this in 4.6 as a bugfix? The script is totally broken this
way.



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] scripts: coccinelle: remove check to move constants to right

[Wolfram Sang]

On Sat, Mar 19, 2016 at 06:43:08PM +0100, Julia Lawall wrote:
> On Sat, 19 Mar 2016, Wolfram Sang wrote:
> 
> > The header mentions this check depends on personal taste. I agree.
> > Running coccicheck on patches before I apply them, this SmPL produced
> > enough false positives for me that I'd rather see it removed.
> 
> An improvement is coming up, that should be more acceptable.  However, 
> it's being held up by the need for some bug fixes in Coccinelle.  A 
> release of Coccinelle is planned for the beginning of April.  Perhaps 
> it is just as well to just remove this version for now.
> 
> Acked-by: Julia Lawall <julia.law...@lip6.fr>

Michal, can we have that in v4.6 please? The false positives are
annoying...

Thanks,

   Wolfram



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] keeping cast affects match?

[Wolfram Sang]

> The isomorphism that allows ignoring a cast only works when the type
> metavariable is used only once.  You uare using it twice.  The fact that
> you wanted to add it indicates that you really want it to be there.

Aha, I understand. Thank you for the explanation!

> I think that everything should be fine if you just move the (T) out of the
> removed and added code.

Much better! I still fall for thinking too much in terms of 'lines'
instead of 'tokens'. Prompt and proper response (again) much
appreciated. Thank you a ton!

   Wolfram



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] keeping cast affects match?

[Wolfram Sang]

Hi,

using Coccinelle v1.04 from Debian:

$ spatch -version
spatch version 1.0.4 with Python support and with PCRE support

The following semantic patch produces a match with current linux-next:

@@
expression table, dev;
type T;
@@
-   (T)of_match_device(table, dev)->data
+   of_device_get_match_data(dev)


$ spatch -sp_file minimal.cocci drivers/gpu/drm/rcar-du/
...
HANDLING: drivers/gpu/drm/rcar-du/rcar_du_drv.c
diff = 
diff -u -p a/rcar_du_drv.c b/rcar_du_drv.c
--- a/rcar_du_drv.c
+++ b/rcar_du_drv.c
@@ -183,7 +183,7 @@ static int rcar_du_load(struct drm_devic
init_waitqueue_head(>commit.wait);
 
rcdu->dev = >dev;
-   rcdu->info = of_match_device(rcar_du_of_table, rcdu->dev)->data;
+   rcdu->info = of_device_get_match_data(rcdu->dev);
rcdu->ddev = dev;
dev->dev_private = rcdu;

However, the match goes away when I want to keep a potential cast (T) which I
need in other places:

@@
expression table, dev;
type T;
@@
-   (T)of_match_device(table, dev)->data
+   (T)of_device_get_match_data(dev)

I wonder why adding (T) to "+" affects the matching. Is this a bug or do I miss
something?

Thanks,

   Wolfram



signature.asc
Description: PGP signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] version 1.0.0

[Wolfram Sang]

On Sun, Apr 19, 2015 at 05:37:21PM +0200, Julia Lawall wrote:
 Coccinelle version 1.0.0 is released.

Congratulations, Julia! And thank you *very much* for all your hard
work!

All the best,

   Wolfram



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] how to debug missing match?

[Wolfram Sang]

On Sun, Dec 21, 2014 at 11:39:14AM +0100, Julia Lawall wrote:
 On Sun, 21 Dec 2014, Wolfram Sang wrote:
 
  
   does not result in anything although three drivers should match IMO with
   the same pattern as the pmu driver above. Both use
   platform_driver_register() and both have the .owner field set.
   
   drivers/macintosh/windfarm_pm112.c
   drivers/macintosh/windfarm_pm72.c
   drivers/macintosh/windfarm_rm31.c
   
   How can I debug why the match does not occur?
  
  So, I found --verbose-parsing to be helpful which gives me:
  
  parse error 
   = File drivers/macintosh/windfarm_pm112.c, line 685, column 4, charpos = 
  17702
  around = 'nr_cores', whole content =++nr_cores;
  
  which basically means that it can't handle for_each_node_by_type()?
  
  bad:for_each_node_by_type(cpu, cpu)
  BAD:!   ++nr_cores;
  
  But neither --include include/linux/of.h nor --recursive-includes
  does help the case for me?
 
 It is not surprising that adding more includes doesn't help.  But I would 
 have thought that for_each_node_by_type, as it begins with for, would be 
 something that it would pick up on.  I will take a look.

How does coccinelle work with such constructs? I'd think that replacing
that #define with the actual for()-construct is much more reliable than
an assumption like it starts with 'for' so it is probably...' :)



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] bugon.cocci: fix Options at the macro

[Wolfram Sang]

On Fri, Nov 07, 2014 at 06:51:05PM +0100, Wolfram Sang wrote:
 On Tue, Oct 28, 2014 at 03:18:24PM -0200, Mauro Carvalho Chehab wrote:
  The comma after --no-includes makes coccinelle to not run the script:
  
  /usr/bin/spatch -D report --very-quiet --no-show-diff --cocci-file 
  ./scripts/coccinelle/misc/bugon.cocci --no-includes, --include-headers 
  --patch . --dir drivers/media/platform/coda/ -I ./arch/x86/include -I 
  arch/x86/include/generated -I include -I ./arch/x86/include/uapi -I 
  arch/x86/include/generated/uapi -I ./include/uapi -I include/generated/uapi 
  -I ./include/linux/kconfig.h
  Usage: spatch.opt --sp-file SP infile [-o outfile] [--iso-file iso] 
  [options]
  Options are:
--sp-filefile the semantic patch file
-o   file the output file
--in-place   do the modification on the file directly
--backup-suffix  suffix to use when making a backup for 
  inplace
  ...
  
  At least with Fedora 20 coccinelle package:
  coccinelle-1.0.0-0.rc20.1.fc21.x86_64
  
  Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 
 Yay, with this my patch handling scripts finally work again as they used
 to! Please let it go into 3.18.
 
 Tested-by: Wolfram Sang w...@the-dreams.de

Ping.



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] bugon.cocci: fix Options at the macro

[Wolfram Sang]

On Tue, Oct 28, 2014 at 03:18:24PM -0200, Mauro Carvalho Chehab wrote:
 The comma after --no-includes makes coccinelle to not run the script:
 
 /usr/bin/spatch -D report --very-quiet --no-show-diff --cocci-file 
 ./scripts/coccinelle/misc/bugon.cocci --no-includes, --include-headers 
 --patch . --dir drivers/media/platform/coda/ -I ./arch/x86/include -I 
 arch/x86/include/generated -I include -I ./arch/x86/include/uapi -I 
 arch/x86/include/generated/uapi -I ./include/uapi -I include/generated/uapi 
 -I ./include/linux/kconfig.h
 Usage: spatch.opt --sp-file SP infile [-o outfile] [--iso-file iso] 
 [options]
 Options are:
   --sp-filefile the semantic patch file
   -o   file the output file
   --in-place   do the modification on the file directly
   --backup-suffix  suffix to use when making a backup for inplace
 ...
 
 At least with Fedora 20 coccinelle package:
   coccinelle-1.0.0-0.rc20.1.fc21.x86_64
 
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Yay, with this my patch handling scripts finally work again as they used
to! Please let it go into 3.18.

Tested-by: Wolfram Sang w...@the-dreams.de



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] sgen problems

[Wolfram Sang]

On Sat, Oct 18, 2014 at 08:02:00AM +0200, Julia Lawall wrote:
  @fix1_context depends on match1  !patch  (context || org || report)@
  identifier match1.__driver;
  position j0;
  @@
  
  *   static struct platform_driver __driver@j0 = {
  .driver = {
  .owner = THIS_MODULE,
  }
  };
  
  @fix2_context depends on match2  !patch  (context || org || report)@
  identifier match2.__driver;
  position j0;
  @@
  
  *   static struct platform_driver __driver@j0 = {
  .driver = {
  .owner = THIS_MODULE,
  }
  };
 
 In these cases, I think it would look nicer to put the *s and the @j0's 
 on .owner rather than at the start of the structure.

Is this a manual fix or a fix to sgen?

 Unless there is some situation in which the transformation is not 
 required, I think High confidence would be appropriate.

The calls I check for always set .owner. So it should be 'high', I'd
guess. However, if the behavioue of these calls are ever to be changed
there is a risk that this spatch gets not updated. More like a general
problem, though, which does not really influence the confidence.



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] sgen problems

[Wolfram Sang]

Hi,

so I was trying to convert my spatch from the platform-owner series to a
Kernel-cocci with sgen. However, that didn't work and produced a corrupt
cocci file (at least the name of the matching rules has been eaten). I
used a self-compiled sgen from the Debian rc22. I attach the cocci and
config file for easier reproducing. Does the sgen author read the list?
Probably it would be easy to fix the broken file, but it seems more
sustainable to me to fix the tool :)

Thanks,

   Wolfram

@match1@
declarer name module_platform_driver;
declarer name module_platform_driver_probe;
identifier __driver;
@@
(
module_platform_driver(__driver);
|
module_platform_driver_probe(__driver, ...);
)

@fix1 depends on match1@
identifier match1.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

@match2@
identifier __driver;
@@
(
platform_driver_register(__driver)
|
platform_driver_probe(__driver, ...)
|
platform_create_bundle(__driver, ...)
)

@fix2 depends on match2@
identifier match2.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};
// Generated config
description = Remove .owner field if calls are used which set it automatically
confidence = Moderate
authors = Wolfram Sang. GPL v2.
fix2 =
  org:No need to set .owner here
  report:No need to set .owner here
fix1 =
  org:No need to set .owner here
  report:No need to set .owner here
/// Remove .owner field if calls are used which set it automatically
///
// Confidence: Moderate
// Copyright: (C) 2014 Wolfram Sang. GPL v2.

virtual patch
virtual context
virtual org
virtual report

@declarer name module_platform_driver;
declarer name module_platform_driver_probe;
identifier __driver;
@@
(
module_platform_driver(__driver);
|
module_platform_driver_probe(__driver, ...);
)

@fix1 depends on match1  patch  !context  !org  !report@
identifier match1.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

@identifier __driver;
@@
(
platform_driver_register(__driver)
|
platform_driver_probe(__driver, ...)
|
platform_create_bundle(__driver, ...)
)

@fix2 depends on match2  patch  !context  !org  !report@
identifier match2.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

// 

@fix1_context depends on match1  !patch  (context || org || report)@
identifier match1.__driver;
position j0;
@@

*   static struct platform_driver __driver@j0 = {
.driver = {
.owner = THIS_MODULE,
}
};

@fix2_context depends on match2  !patch  (context || org || report)@
identifier match2.__driver;
position j0;
@@

*   static struct platform_driver __driver@j0 = {
.driver = {
.owner = THIS_MODULE,
}
};

// 

@script:python fix1_org depends on org@
j0  fix1_context.j0;
@@

msg = No need to set .owner here.
coccilib.org.print_todo(j0[0], msg)

@script:python fix2_org depends on org@
j0  fix2_context.j0;
@@

msg = No need to set .owner here.
coccilib.org.print_todo(j0[0], msg)

// 

@script:python fix1_report depends on report@
j0  fix1_context.j0;
@@

msg = No need to set .owner here.
coccilib.report.print_report(j0[0], msg)

@script:python fix2_report depends on report@
j0  fix2_context.j0;
@@

msg = No need to set .owner here.
coccilib.report.print_report(j0[0], msg)

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

Re: [Cocci] [RFC] drop owner assignment from platform_drivers

[Wolfram Sang]

  So, what is your opinion on the original cleanup series removing
  unnecessary '.owner = THIS_MODULE' lines in drivers? Helpful? Noise?
 
 Helpful, please do it.  I can take it all through my driver-core tree if
 you want, that might make things easier for others.

Thanks, that might make sense this time.

So, I'll prepare the bugfixes, add the semantic patch, add this all to
my series and respin. It might take a few days because of ELCE in
Düsseldorf, but I'll certainly do it.



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [RFC] drop owner assignment from platform_drivers

[Wolfram Sang]

  You got me wondering, though, that it could not be correct to call
  platform_driver_register() from the platform core instead of module
  init. I will check tomorrow. Still, this would be a bug independent of
  my series. Although I'd need to respin it if platform_driver_probe()
  needed a fix.
 
 Right, this seems to be a preexisting bug. platform_create_bundle 
 and platform_driver_probe will both overwrite the .owner field with
 NULL since they live in builtin code. They need to be replaced with
 __platform_driver_probe and __platform_driver_register that both
 take an extra owner argument passed down from the caller in the driver
 module.

Yeah, that would be one solution. However, my personal favourite would
meanwhile be to revert the commit that Russell mentioned. I think it is
cleaner to have the owner explicitly set in the module rather than
hidden away by a function call. However, grepping through include/linux,
there are a few subsystems hiding it this way. So, it is a pattern
somewhow. Oh well...



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] [RFC] drop owner assignment from platform_drivers

[Wolfram Sang]

Hi,

people found out that for platform_driver, we don't need to set the
.owner field because this is done by the platform driver core. So far,
so good. However, now I got patches removing the .owner field for this
single i2c driver or for that one. To prevent getting thousands of
patches fixing single drivers, I used coccinelle to remove all instances
from the kernel. The SmPL looks like this, it doesn't blindly remove all
THIS_MODULE, but checks if the platform_driver struct was really used by
a call actually setting the .owner field:

===

@match1@
declarer name module_platform_driver;
declarer name module_platform_driver_probe;
identifier __driver;
@@
(
module_platform_driver(__driver);
|
module_platform_driver_probe(__driver, ...);
)

@fix1 depends on match1@
identifier match1.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

@match2@
identifier __driver;
@@
(
platform_driver_register(__driver)
|
platform_driver_probe(__driver, ...)
|
platform_create_bundle(__driver, ...)
)

@fix2 depends on match2@
identifier match2.__driver;
@@
static struct platform_driver __driver = {
.driver = {
-   .owner = THIS_MODULE,
}
};

===

I tried to group the changes. The current granularity is directory
level. The resulting branch can be found here (it is based on linux-next
of yesterday):

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git 
drop_platform_driver_owner

This still results in 300 patches as this shortened pull request shows:

===

Wolfram Sang (299):
  ARM: common: drop owner assignment from platform_drivers
  ARM: mach-davinci: drop owner assignment from platform_drivers
  ARM: mach-imx: drop owner assignment from platform_drivers
...
  ASoC: txx9: drop owner assignment from platform_drivers
  ASoC: ux500: drop owner assignment from platform_drivers
  ALSA: sparc: drop owner assignment from platform_drivers

 arch/arm/common/sa.c | 1 -
 arch/arm/mach-davinci/cpuidle.c  | 1 -
 arch/arm/mach-davinci/pm.c   | 1 -
...
 sound/sparc/amd7930.c| 1 -
 sound/sparc/cs4231.c | 1 -
 sound/sparc/dbri.c   | 1 -
 1688 files changed, 1718 deletions(-)

===

I don't want to send all 300 patches to lkml. I still think, they should
go via subsystems, though, and not via a single pull request. So, I am
working on just sending smaller pieces of this huge series to the
apropriate mailing lists (like arm, netdev...) as an independent series.
Then, each subsystem can decide if the patches should be further folded.
In my experience, this mileage varies from subsystem to subsystem.

That's my plan for today. If you have comments, other suggestions or
remarks, I'd like to hear them.

Thanks,

   Wolfram



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [RFC] drop owner assignment from platform_drivers

[Wolfram Sang]

 The semantic patch looks fine.

Wow, nothing to improve on the semantic patch? Now I am proud :) Thanks
Julia for your support, as always!

 If you think that it would be useful to have this in the Linux kernel, so
 people don't add the owner initializer back in the future, you can try
 
 coccinelle/tools/sgen/sgen

Will try later this weekend, thanks!



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [RFC] drop owner assignment from platform_drivers

[Wolfram Sang]

Hi Arnd,

thanks for taking a look!

On Fri, Oct 10, 2014 at 10:30:08AM +0200, Arnd Bergmann wrote:
 On Friday 10 October 2014 09:24:39 Wolfram Sang wrote:
  people found out that for platform_driver, we don't need to set the
  .owner field because this is done by the platform driver core. So far,
  so good. However, now I got patches removing the .owner field for this
  single i2c driver or for that one. To prevent getting thousands of
  patches fixing single drivers, I used coccinelle to remove all instances
  from the kernel. The SmPL looks like this, it doesn't blindly remove all
  THIS_MODULE, but checks if the platform_driver struct was really used by
  a call actually setting the .owner field:
 
 Is the intention just to save a few lines in the kernel source, or are
 there any additional upsides to doing this?

As written above, I don't like getting patches removing this line for
single drivers. I already got two and I am expecting more. So I'd prefer
to do this on subsystem level. I will apply the I2C part, for sure.

 While it looks like an obvious cleanup, it also seems to me that there
 is zero effect in terms of functionality, code size or enabling future
 changes.

Well, the kernel image will compress better ;) And well, it is cleaner.
Why should we set up something if it gets overwritten anyhow?

 I'm all for adding your semantic patch to scripts/coccinelle so it gets
 picked up by anyone writing new drivers or doing code cleanup on their
 driver, but I'm unsure about the value of applying all your patches
 for the existing drivers.

I could try reducing the number of patches. Any other downsides?

Thanks,

   Wolfram


signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [RFC] drop owner assignment from platform_drivers

[Wolfram Sang]

On Fri, Oct 10, 2014 at 09:36:27AM +0100, Russell King - ARM Linux wrote:
 On Fri, Oct 10, 2014 at 09:24:39AM +0200, Wolfram Sang wrote:
  people found out that for platform_driver, we don't need to set the
  .owner field because this is done by the platform driver core. So far,
  so good. However, now I got patches removing the .owner field for this
  single i2c driver or for that one. To prevent getting thousands of
  patches fixing single drivers, I used coccinelle to remove all instances
  from the kernel. The SmPL looks like this, it doesn't blindly remove all
  THIS_MODULE, but checks if the platform_driver struct was really used by
  a call actually setting the .owner field:
 
 Is this correct?
 
 #define platform_driver_register(drv) \
 __platform_driver_register(drv, THIS_MODULE)
 extern int __platform_driver_register(struct platform_driver *,
 struct module *);
 
 Fine for those which use platform_driver_register(), but:
 
 /* non-hotpluggable platform devices may use this so that probe() and
  * its support may live in __init sections, conserving runtime memory.
  */
 extern int platform_driver_probe(struct platform_driver *driver,
 int (*probe)(struct platform_device *));
 
 platform_driver_probe() doesn't seem to know which module called it.
 This is also true of platform_create_bundle:
 
 extern struct platform_device *platform_create_bundle(
 struct platform_driver *driver, int (*probe)(struct platform_device 
 *),
 struct resource *res, unsigned int n_res,
 const void *data, size_t size);
 
 So, it's not as trivial as just all platform driver's should not have a
 .owner field - the real answer is far more complex than that.

platform_create_bundle() calls platform_driver_probe().
platform_driver_probe() calls platform_driver_register().
platform_driver_register() modifies driver.owner.

So, it is correct from the point of view that it doesn't make sense to
set the .owner field if it gets overwritten anyhow.

You got me wondering, though, that it could not be correct to call
platform_driver_register() from the platform core instead of module
init. I will check tomorrow. Still, this would be a bug independent of
my series. Although I'd need to respin it if platform_driver_probe()
needed a fix.

Thanks,

   Wolfram



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Documentation generation for the release 1.0.0-rc22

[Wolfram Sang]

 I have searched a bit in a few archives.

Really? This is what my search found in approx. 3 seconds (assuming
SUSE):

http://rpm.pbone.net/index.php3/stat/3/srodzaj/1/search/tex%28ptmr8t.tfm%29



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Documentation generation for the release 1.0.0-rc22

[Wolfram Sang]

 The tool RPM PBone Search was not on my radar screen for a while.

It was the first hit for me when it googled suse and the font name :)

Anyway, glad it works now...



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH 1/1] scripts/coccinelle: use BIT() macro if possible

[Wolfram Sang]

On Sun, Apr 27, 2014 at 02:29:46AM +0200, Javier Martinez Canillas wrote:
 Using the BIT() macro instead of manually shifting bits
 makes the code less error prone and also more readable.

Does it? It is a taste thing, yet I don't think it makes the code that
much more readable that it is worth changing the whole tree.



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] [bug] file not found with id-utils and include-headers

[Wolfram Sang]

Hi,

using the latest Debian package from testing (rc18) and the following
spatch on the Linux Kernel:

@@
expression e;
@@
-   INIT_COMPLETION(e);
+   reinit_completion(e);


Running with:

spatch --sp-file /tmp/reinit_completion.cocci --use-idutils --include-headers ./

I get:

init_defs_builtins: /usr/share/coccinelle/standard.h
got files
HANDLING: INIT_COMPLETION drivers/hid/hid-wiimote.h
egrep: INIT_COMPLETION: No such file or directory
EXN:Sys_error(INIT_COMPLETION drivers/hid/hid-wiimote.h: No such file or 
directory)
...

The following files are processeed correctly. This call works on the
file which fails above:

spatch --sp-file /tmp/reinit_completion.cocci --include-headers ./drivers/hid/

I hope you can reproduce.

Regards,

   Wolfram



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [PATCH] scripts/coccinelle/api: remove devm_request_and_ioremap.cocci

[Wolfram Sang]

On Thu, Aug 15, 2013 at 12:30:25PM +0200, Wolfram Sang wrote:
 Use of this function is discouraged in favour of
 devm_ioremap_resource(). Don't advertise it.
 
 Signed-off-by: Wolfram Sang w...@the-dreams.de

Ping.



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] if statements including assignments

[Wolfram Sang]

Hi,

I'd basically need something like:

if (... when != var)

to match. I couldn't find a way to express it. This is needed since
there are constructs like:

if (var = something())

which I don't want to match since they assign var a value which is fine
in my case.

Thanks,

   Wolfram



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] if statements including assignments

[Wolfram Sang]

  I'd basically need something like:
  
  if (... when != var)
 
 This is not really clear enough for me to understand what you do want to 
 do.  I imagine that you want something like
 
 (
 if (pattern_you_like) S1 else S2
 |
 * if (pattern_you_dont_like)
   S1 else S2
 )

Ah, so I was on a good path since I ended up with something similar...

 Where pattern_you_like might be +... var ...+, if you like everything 
 that somewhere contains var.

... and this hint made it work, thanks a lot! Before that I was using

if (var = ...) S

as pattern_I_like. But that never matched and thus always ended up in
the pattern_i_dont_like branch.

 I would really rather that all assignments in if tests disappear,

+1. They are often annoying.

 the if, but it touched over 1100 files in the Linux kernel, so I ran out 
 of courage...

:)

Thanks,

   Wolfram



signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Removing pointer declaration

[Wolfram Sang]

Hi,

This spatch:

@@
int *p;
@@

-   int *p;

with this code:


int main()
{
int *a;

return 0;
}

results in:

Fatal error: exception Failure(minus: parse error: 
 = File test.cocci, line 5, column 7,  charpos = 22
around = 'p', whole content = - int *p;
)

using:

spatch version 1.0.0-rc15 with Python support and with PCRE support
(although dpkg says it is rc16)

Am I doing something wrong?

Thanks,

   Wolfram


signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] id-utils script?

[Wolfram Sang]

Hi,

I wanted to try id-utils in comparison with grep, but I can't find the
script 'idindex-cocci.sh' mentioned in the doc files. Not even in older
releases. Does someone still have it? Is id-utils still supported? And
while I am here, any outcome of the suggestion to use cscope?

Thanks,

   Wolfram

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

[Cocci] id-utils?

[Wolfram Sang]

Hi,

I wanted to try the --id-utils option to see what the speedup was like.
The docs mention a script called 'idindex-cocci.sh' to generate the
index but I am unable to find it (no luck in older version of
coccinelle, too). Does someone still have it? And in one older mail
thread cscope was mentioned as an alternative. Has this been tried
meanwhile?

Thanks,

   Wolfram


signature.asc
Description: Digital signature
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] isomorphism with unlikely

[Wolfram Sang]

Hi,

using 1.0-rc16 from debian, I use the following semantic patch:

@@
expression r, p, n;
@@
r = platform_get_resource(p, IORESOURCE_MEM, n);
...
(
-   if (!r) { ... }
|
-   if (unlikely(!r)) { ... }
|
-   if (unlikely(r == NULL)) { ... }
)

and running against the latest kernel tree (~3.10-rc1):

spatch --sp-file unlikely_iso.cocci 
~/Kernel/linux/drivers/net/ethernet/renesas/sh_eth.c

gives me a match:

-   if (unlikely(res == NULL)) {
...

Note that I have two alternations with 'unlikely' because using '!r'
alone will not give me a match. I guess the isomorphism is skipped for
some reason when used as an argument of unlikely. If so, is it possible
to enforce it somehow? Or would it be even better to make 'unlikely' an
isomorphism itself, so that the semantic patch doesn't need to know
anything about 'unlikely'?

Thanks,

   Wolfram

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

Re: [Cocci] isomorphism with unlikely

[Wolfram Sang]

 There is already an isomorphism for unlikely. But it is in one direction
 only, so 'unlikely(E)' in your cocci script will also match 'E', but 'E'
 wont match 'unlikely(E)'.

Okay, but I would then still need 'unlikely(!r)' and 'unlikely(r ==
NULL)' because the !r isomorphisms won't apply. I'd like to have both :)

 Btw. if you use 'statement S;' and 'if(...) S' instead of '{ ... }' you'll
 also be able to match if there is only a single statement without brackets
 after the if.

True, thanks! Slowly getting back to it...

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