On Sat, May 14, 2011 at 1:17 PM, Nicolas Palix <[email protected]> wrote:
> 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.

ack.  I tried to bind them to the md rule,
both in metavar section, and in transform section.
ie
expression md.ct, md.name;
-register_chrdev_region(devt, md.ct, md.name)

the errors suggested that was wrongheaded.


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