[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8 02/10] scripts: add coccinelle script to use auto propagated errp
Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes: > 11.03.2020 12:38, Markus Armbruster wrote: >> Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes: >> >>> 09.03.2020 12:56, Markus Armbruster wrote: >>>> Suggest >>>> >>>> scripts: Coccinelle script to use auto-propagated errp >>>> >>>> or >>>> >>>> scripts: Coccinelle script to use ERRP_AUTO_PROPAGATE() >>>> >>>> Vladimir Sementsov-Ogievskiy <vsementsov@xxxxxxxxxxxxx> writes: >> [...] >>>>> +// Note, that we update everything related to matched by rule1 function >>>>> name >>>>> +// and local_err name. We may match something not related to the pattern >>>>> +// matched by rule1. For example, local_err may be defined with the same >>>>> name >>>>> +// in different blocks inside one function, and in one block follow the >>>>> +// propagation pattern and in other block doesn't. Or we may have several >>>>> +// functions with the same name (for different configurations). >>>> >>>> Context: rule1 matches functions that have all three of >>>> >>>> * an Error **errp parameter >>>> >>>> * an Error *local_err = NULL variable declaration >>>> >>>> * an error_propagate(errp, local_err) or error_propagate_prepend(errp, >>>> local_err, ...) expression, where @errp is the parameter and >>>> @local_err is the variable. >>>> >>>> If I understand you correctly, you're pointing out two potential issues: >>>> >>>> 1. This rule can match functions rule1 does not match if there is >>>> another function with the same name that rule1 does match. >>>> >>>> 2. This rule matches in the entire function matched by rule1, even when >>>> parts of that function use a different @errp or @local_err. >>>> >>>> I figure these apply to all rules with identifier rule1.fn, not just >>>> this one. Correct? >>>> >>>> Regarding 1. There must be a better way to chain rules together, but I >>>> don't know it. >>> >>> Hmm, what about something like this: >>> >>> @rule1 disable optional_qualifier exists@ >>> identifier fn, local_err; >>> symbol errp; >>> @@ >>> >>> fn(..., Error ** >>> - errp >>> + ___errp_coccinelle_updating___ >>> , ...) >>> { >>> ... >>> Error *local_err = NULL; >>> ... >>> ( >>> error_propagate_prepend(errp, local_err, ...); >>> | >>> error_propagate(errp, local_err); >>> ) >>> ... >>> } >>> >>> >>> [..] >>> >>> match symbol ___errp_coccinelle_updating___ in following rules in function >>> header >>> >>> [..] >>> >>> >>> @ disable optional_qualifier@ >>> identifier fn, local_err; >>> symbol errp; >>> @@ >>> >>> fn(..., Error ** >>> - ___errp_coccinelle_updating___ >>> + errp >>> , ...) >>> { >>> ... >>> } >>> >>> >>> - hacky, but seems not more hacky than python detection, and should work >>> better >> >> As simple, forceful and unsubtle as a sledgehammer. I like it :) >> > > > Hmm, not so simple. > > It leads to reindenting of function header, which is bad. Because ___errp_coccinelle_updating___ is longer than errp, I guess. Try ____? > Possible solution is instead > > fn(...) > { > + ___errp_coccinelle_updating___(); > > > but this slow down coccinelle. For example, on block.c from ~3s to 1m16s. > > . > > So, I'm returning to just a warning. > > I think something simple like > > @@ > identifier rule1.fn; > position p != rule1.p; > @@ > > fn@p(...) {...} > > @ script:python@ > > <print warning> > > should work. Up to you. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |