On Fri, 13 May 2011, Jim Cromie wrote:
> hi folks,
>
> I need help building my cocci-fu, currently Id have to call it cocci-oof
>
> Ive prepped a patchset, to:
>
> register_chrdev_ids() aims to replace and deprecate
> register_chrdev_region() and alloc_chrdev_region()
> with a single function that works for both dynamic
> and static major numbers.
>
> and am triyng to use spatch to do the user-callsite changes.
> I have partial success, in that Ive gotten some rules to apply,
>
> 0 - is there a way to completely disable a rule with compile-syntax errors,
> such that it doesnt have to be edited out ? /* ... */ doesnt work.
> is there an __END__ ala perl or could there be ?
Sorry about this. Originally we had the idea that // comments would be
SmPL comments and /* */ comments would be added code. But there is no
reason for this distinction. We can just consider an unannotated comment
to be a SmPL comment, and either kind of comment annotated to be + to be
added code. Indeed, this was already supported for //. I have fixed
the treatment of /* */ to allow it to appear without a + in front of it.
In the meantime, you can use //.
> 1 - these 2 rules apply, but are simplistic, and wrong in some cases
> (they disregard the if-then-else that should be removed)
>
> My question with them is around the dev_t.
> 1st 'type's it, 2nd doesnt, both then use 'dev_t'
> where a keyword {expression, identifier, etc} would go,
> this doesnt blow up - whats the real interpretation ??
>
> @ register_chrdev_region @ // depends on fs_h @
> type dev_t;
This is declaring dev_t to be a metavariable representing a type. You
could then for example use this metavariable in some variable declarations
in the SmPL code.
> //identifier devid;
> dev_t devid;
Now that dev_t is a metavariable, this will allow devid to have any type
(or any type compatible with the metavariable value if it had a reason to
have another one, but you don't use dev_t, so that is not your case).
The declaration you wanted was not type, but typedef. But SmPL should be
able to figure out that dev_t should be a type, so you don't need the
typedef in this case either. Here are some examples. In the second
example, the typedef is needed, because the parser can't figure out that
it should be a type itself. Once something has been declared using
typedef, it remains considered that way for the entire semantic patch, not
just for the current rule.
@@
dev_t c;
@@
-foo(c);
@@
typedef xxx_t;
@@
- (xxx_t)y;
> expression ct;
> identifier name;
> @@
>
> - register_chrdev_region(devid, ct, name)
> + register_chrdev_ids(&devid, ct, name)
>
> @ alloc_chrdev_region @ // depends on fs_h @
>
> // type dev_t;
> dev_t devid;
> expression minor; // ident didnt pick up constants
Normal. If you want constants, you can say constant minor;. That will
match eg 0, and also match identifiers written entirely in upper case. If
you really want all possible identifier and all possible constants, you
can say:
@@
constant c;
identifier i;
@@
foo ( \(i\|c\) )
> expression ct;
> identifier name;
> @@
>
> - alloc_chrdev_region(&devid, minor, ct, name)
> + register_chrdev_ids(&devid, ct, name)
>
>
> 2- this rule wont apply - and is where Im mostly looking atm.
>
> @ for_rtc_dev @ // ./drivers/rtc/rtc-dev.c
>
> // typedef dev_t; // "meta: parse error: around = 'dev_t'
Probably it already figured out that dev_t should be a typedef.
> identifier err;
> identifier rtc_devt;
> expression maj, ct, nm;
> @@
> // dev_t rtc_devt; // < -- VARY THIS LINE
> ... // this doesnt matter
> // err = alloc_chrdev_region(&rtc_devt, 0, RTC_DEV_MAX, "rtc");
> - err = alloc_chrdev_region(&rtc_devt, maj, ct, nm);
> + err = register_chrdev_ids(&rtc_devt, maj, ct, nm);
> if (err < 0) { ... }
>
>
> linux-2.6.git]$ spatch -sp_file ../chrdev.cocci -partial_match
> ./drivers/rtc/rtc-dev.c
>
> ... (other rules elided) ...
>
> HANDLING: ./drivers/rtc/rtc-dev.c
> Empty list of bindings, I will restart from old env
> Empty list of bindings, I will restart from old env
> Empty list of bindings, I will restart from old env
> Empty list of bindings, I will restart from old env
> Empty list of bindings, I will restart from old env
> partial matches
> 7[]{!wit 7[err --> id err]
> {wit 7[rtc_devt --> id rtc_devt]
> {wit 7[maj --> 0]
> {wit 7[ct --> RTC_DEV_MAX]
> {wit 7[nm --> "rtc"]
> {wit 7[_v --> for_rtc_dev:err = alloc_chrdev_region(
> &for_rtc_dev:rtc_devt, for_rtc_dev:maj,
> for_rtc_dev:ct, for_rtc_dev:nm);]{}}}}}}}
>
>
> Empty list of bindings, I will restart from old env
>
>
> If I remove the leading // on the <-- VARY THIS LINE above,
> I get no partial match, only 1 more empty-list-of-bindings.
>
> This rule has 5 metavars, all are mentioned in the partial-matching output,
> whats missing ?
I have the impression that it found the call to alloc_chrdev_region, but
not the subsequent if (err < 0) test. For that you have written { ... },
but in the code there are no braces. I usually put a statement
metavariable in this context. That will match a single statement by
itself as well as anything with braces.
There are isomorphisms for eg { ... S } => S, but nothing for braces with
nothing specified inside, so that would mean that the braces really have
to be there.
> I have 6 rules, and 6 empty-bindings, which seems to make sense.
>
>
>
> 3- this rule does work, at least in one case.
> If I can generalize it properly, I might be finished...
>
>
>
> @ for_fuse_cuse @ // this works
> identifier rc, devt;
> expression arg1, arg2; // doesnt work as ident - lh_expr would if it existed.
> expression MIN, CT, devname;
> expression warn_on_this_expr;
> identifier warn_on_this_ident;
> @@
>
> // devt = MKDEV(arg->dev_major, arg->dev_minor);
> devt = MKDEV(arg1, arg2);
> ... when != devt // no harm to purpose, may help other apps?
> - if (!MAJOR(devt))
> //- rc = alloc_chrdev_region(&devt, MINOR(devt), 1,
> devinfo.name);
> - rc = alloc_chrdev_region(&devt, MIN, CT, devname);
> - else
> //- rc = register_chrdev_region(devt, 1, devinfo.name);
> - rc = register_chrdev_region(devt, CT, devname);
> + rc = register_chrdev_ids(&devt, CT, devname);
> if (rc) {
> ...
> }
Could you send an example of something you hope to match?
julia
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)
