On Sat, 14 May 2011, Jim Cromie wrote:
> On Sat, May 14, 2011 at 10:36 AM, Julia Lawall <[email protected]> wrote:
> > On Sat, 14 May 2011, Jim Cromie wrote:
> >
> >> >> drivers/tty/tty_io.c: unregister_chrdev_region(MKDEV(driver->major,
> >> >> driver->minor_start),
> >> >> drivers/tty/tty_io.c: register_chrdev_region(MKDEV(TTYAUX_MAJOR,
> >> >> 0), 1, "/dev/tty") < 0)
> >> >> drivers/tty/tty_io.c: register_chrdev_region(MKDEV(TTYAUX_MAJOR,
> >> >> 1), 1, "/dev/console") < 0)
> >> >>
>
> ok. from what I understood of your responses, I tried the following:
>
> has +code before -code, inserting into position @pdecl
> -code should be providing the matched expr for the +code.
> spatch didnt complain, so I guess its legal.
As long as + code has something to attach to everything is find. This can
just be a {.
>
>
> @ mkdev_expr @
> dev_t devid;
> expression ct, name;
> expression major, minor;
> position pdecl;
> @@
>
> { @pdecl
> + dev_t devt = MKDEV(major,minor);
> ...
> - register_chrdev_region(MKDEV(major,minor), ct, name);
> + register_chrdev_region(devt, ct, name);
Here, you are replacing one function call by another. If it's useful, you
could drop the ; in both cases, to replace an expression by an expression,
rather than a statement by a statement.
> ...
> }
>
> when used as written, it misses the expr inside the if statement,
> However, if I strip the semicolons off the -+ code, to allow expression
> match rather than statement match, the rule hangs indefinitely.
>
> ^C C-c intercepted, will do some cleaning before exiting
Hmm. On what file? You can also use the argument -show_trying to find
out what function is it working on, and -debug to find out what rule in
the semantic patch it is working on.
> here another run, using a different expression-transform
> on one of the more complicated call-sites.
>
> @ register_chrdev_region @ // depends on fs_h @
> //dev_t devid;
> expression devid;
> expression ct, name;
> @@
>
> - register_chrdev_region(devid, ct, name)
> + register_chrdev_ids(&devid, ct, name)
>
>
>
> [jimc@groucho linux-2.6.git]$ spatch -sp_file chrdev.cocci-expr
> drivers/tty/tty_io.c -partial_match -debug
You should only use -partial_match as a last resort. In general it will
make things much more expensive.
> ....
> ----------------------------------------------------------------------
> Finished
> -----------------------------------------------------------------------
> diff =
> --- drivers/tty/tty_io.c 2011-05-14 00:52:22.221529714 -0600
> +++ /tmp/cocci-output-4815-63ba00-tty_io.c 2011-05-14 11:20:22.293249506
> -0600
> @@ -3295,13 +3295,13 @@ int __init tty_init(void)
> {
> cdev_init(&tty_cdev, &tty_fops);
> if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
> - register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
> + register_chrdev_ids(&MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
> panic("Couldn't register /dev/tty driver\n");
> device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 0), NULL, "tty");
>
> cdev_init(&console_cdev, &console_fops);
> if (cdev_add(&console_cdev, MKDEV(TTYAUX_MAJOR, 1), 1) ||
> - register_chrdev_region(MKDEV(TTYAUX_MAJOR, 1), 1, "/dev/console") <
> 0)
> + register_chrdev_ids(&MKDEV(TTYAUX_MAJOR, 1), 1, "/dev/console") < 0)
> panic("Couldn't register /dev/console driver\n");
> consdev = device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 1), NULL,
> "console");
> Check duplication for 1 files
>
>
> Ideally, my rule would catch and transform all 4 (2x2) occurrences of
> MKDEV(TTYAUX_MAJOR, [01]), but in this particular case,
> a single decl wouldnt suffice; 2 are needed:
> dev_t ttydev0 = MKDEV(TTYAUX_MAJOR, 0);
> dev_t ttydev1 = MKDEV(TTYAUX_MAJOR, 1);
>
> this is rather more binding-fu than Ive got or need,
> but if generalized by a master, could probably hoist
> a lot of invariant code out of inner scopes.
> Runtime might be a problem..
You can use ++ instead of + to allow added things to accumulate. You have
no control over the order they come out in, though.
You will need to put when any on each of the ...s in the { }. Without
that:
{
...
foo();
...
}
actually means:
{
... when != foo();
foo();
... when != foo();
}
That is, the pattern before or after a ... should not appear within the
...
On the other hand, the when any is not a good idea either, because it will
allow {'s as well, and then you won't get the innermost braces.
A nest would work as well:
{
+ your decl
<...
foo(x);
...>
}
> Given that my more constrained version above is hanging,
> I cant cast a wider net, but heres what I was hoping to do.
>
>
>
>
> @ md @
> expression ct, name;
> expression major, minor;
> position pdecl;
> @@
>
> { @pdecl
> + dev_t devt = MKDEV(major,minor);
> ...
> - MKDEV(major,minor)
> + devt
> ...
> }
If no other rules is using pdecl, you don't need it. A position variable
is only useful to ensur that two things are in the same position, or to
ensure that two things are in different positions (position p1 != r.p2).
> @@
> dev_t devt;
> @@
>
> - register_chrdev_region(devt, ct, name)
> + register_chrdev_ids(&devt, ct, name)
>
>
> Oof - Ive asked -parse_cocci questions regarding this in other reply..
> the ones about identiffffier foob belong there,
> but these did not..
>
>
> 2 - how to fix this warning ?
>
> warning: iso braces2 does not match the code below on line 9
> {
> >>> dev_t devt = MKDEV(md:major, md:minor);
I don't think you want to fix it. What it is saying is that the
isomorphism { ... S ... } => S doesn't apply. This is because if the { }
aren't there, then it doesn't know where to put the added declaration.
But since you want the added declaration to be added just after the
innermost brace, you want the braces to be there.
julia_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)
