On Sun, May 15, 2011 at 2:04 AM, Julia Lawall <[email protected]> wrote:
> What are the two places you want to change?
2 kinds of changes: (both to drivers/tty/tty_io.c: tty_init)
1 register_chardev_region --> register_chardev_ids
with that was MKDEV --> &devt_X
That transform is working.
using ++ instead of +, I was able to get this out,
which is useful as starting point for hand editing.
One thing Id like to add is a _0 and _1 suffix
to the 2 new varnames, using the bound values of minor
would be useful in this case.
int __init tty_init(void)
{
+ dev_t devt;
+ devt = MKDEV(TTYAUX_MAJOR, 0);
+ dev_t devt;
+ devt = MKDEV(TTYAUX_MAJOR, 1);
+ dev_t devt;
+ devt = devt;
+ dev_t devt;
+ devt = devt;
2 the 2nd KIND of change: MKDEV(...) --> devt
there are other occurrences of MKDEV in tty_init(),
which shouldnt have '&' in the replacement
In the patch below, these added changes were done by a 2nd,
separate rule which was not constrained by 'register_chrdev_region',
but depended upon it, so it wasnt touching things inappropriately.
This is probably the better way to do it.
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)
+ if (cdev_add(&tty_cdev, devt, 1) ||
+ register_chrdev_ids(&devt, 1, "/dev/tty") < 0)
panic("Couldn't register /dev/tty driver\n");
- device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 0), NULL, "tty");
+ device_create(tty_class, NULL, devt, 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)
+ if (cdev_add(&console_cdev, devt, 1) ||
+ register_chrdev_ids(&devt, 1, "/dev/console") < 0)
panic("Couldn't register /dev/console driver\n");
- consdev = device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 1), NULL,
+ consdev = device_create(tty_class, NULL, devt, NULL,
"console");
if (IS_ERR(consdev))
consdev = NULL;
Note: processing took 93.4s: drivers/tty/tty_io.c
Again, the possible improvements are
- single var decl at top,
- multiple assignments to it, just above 1st use.
Making this not do harm is nice but ultimately overkill,
I need to review before applying anyway.
> It is never useful to put <+... ...+> around an entire rule. That is what
> the semantics of rule application already does naturally.
OK. that makes sense.
The reason they were in there was due to my attempt to also convert
the 2nd Kind of MKDEV in the same rule.
the inner <+... ...+> was trying to change them, using the outer
transform as an anchor. Part of my thinking here was that I wanted
to insert assgments to the devt var just above the 2 if()s where
the MKDEVs were.
ie to produce this:
int __init tty_init(void)
{
+ dev_t devt;
cdev_init(&tty_cdev, &tty_fops);
+ devt = MKDEV(TTYAUX_MAJOR, 0);
- if (cdev_add(&tty_cdev, MKDEV(TTYAUX_MAJOR, 0), 1) ||
- register_chrdev_region(MKDEV(TTYAUX_MAJOR, 0), 1, "/dev/tty") < 0)
+ if (cdev_add(&tty_cdev, devt, 1) ||
+ register_chrdev_ids(&devt, 1, "/dev/tty") < 0)
panic("Couldn't register /dev/tty driver\n");
- device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 0), NULL, "tty");
+ device_create(tty_class, NULL, devt, NULL, "tty");
cdev_init(&console_cdev, &console_fops);
+ devt = MKDEV(TTYAUX_MAJOR, 1);
- if (cdev_add(&console_cdev, MKDEV(TTYAUX_MAJOR, 1), 1) ||
- register_chrdev_region(MKDEV(TTYAUX_MAJOR, 1), 1, "/dev/console") <
0)
+ if (cdev_add(&console_cdev, devt, 1) ||
+ register_chrdev_ids(&devt, 1, "/dev/console") < 0)
panic("Couldn't register /dev/console driver\n");
- consdev = device_create(tty_class, NULL, MKDEV(TTYAUX_MAJOR, 1), NULL,
+ consdev = device_create(tty_class, NULL, devt, NULL,
"console");
if (IS_ERR(consdev))
>
> If you want to add at declaration at the very top of a function, make a
> rule that looks like the function definition and put the declaration
> there. Eg:
ack. I used this , with ++, to insert multiple declarations above.
> Unfortunately, I'm not on line very much today.
>
> julia
Thanks for all your help.
At this point, Im happy with my results. While further refinements to my
rules are likely possible, and useful for my spatch-fu, I can quickly
hand edit the useful changes, reset the others.
If anything about my former (current?) confusion is interesting,
please ask, otherwise thanks for a powerful tool, and the personal help.
jimc
btw, my cocci input
@ rcr_md @
identifier f;
expression major, minor;
expression ct, name;
@@
f(...) {
++ dev_t devt;
++ devt = MKDEV(major,minor);
<+...
- register_chrdev_region
+ register_chrdev_ids
(
- MKDEV(major,minor),
+ &devt,
ct, name)
...+>
}
@ all_md depends on rcr_md @ // where above changes made, also do
identifier f;
expression major, minor;
@@
f(...) {
dev_t devt;
devt = MKDEV(major,minor);
<+...
- MKDEV(major,minor)
+ devt
...+>
}
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)