On Sat, May 14, 2011 at 8:32 PM, Jim Cromie <[email protected]> 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.
>
>
> @ 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);
>  ...
> }

You are introducing a identifier (devt). So, you should not
declare the metavariable devid but a fresh identifier devt.

>
> 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
>
>
>
>
> 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
> ....
> ----------------------------------------------------------------------
> 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..
>
> 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
>  ...
> }

Do you use the position metavariable somewhere ?
You could remove it otherwise.

>
> @@
> dev_t devt;
> @@
>
> - register_chrdev_region(devt, ct, name)
> + register_chrdev_ids(&devt, ct, name)
>

ct and name should be metavariable I guess.

>
> 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);
> _______________________________________________
> Cocci mailing list
> [email protected]
> http://lists.diku.dk/mailman/listinfo/cocci
> (Web access from inside DIKUs LAN only)
>



-- 
Nicolas Palix
http://sardes.inrialpes.fr/~npalix/
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)

Reply via email to