On Tue, Feb 13, 2018 at 10:52 AM, René Scharfe <l.s.r@xxxxxx> wrote: > Am 12.02.2018 um 22:48 schrieb Junio C Hamano: >> René Scharfe <l.s.r@xxxxxx> writes: >> >>> Am 12.02.2018 um 22:04 schrieb Junio C Hamano: >>>> Stefan Beller <sbeller@xxxxxxxxxx> writes: >>>> >>>>> I thought it may be a helpful >>>>> for merging this series with the rest of the evolved code base which >>>>> may make use of one of the converted functions. So instead of fixing >>>>> that new instance manually, cocinelle could do that instead. >>>> >>>> Having the .cocci used for the conversion *somewhere* would indeed >>>> be helpful, as it allows me to (1) try reproducing this patch by >>>> somebody else using the file and following the steps in order to >>>> audit this patch and (2) catch new places that need to be migrated >>>> in in-flight topics. >>>> >>>> But placing it in contrib/coccinelle/ has other side effects. >>> >>> Running "make coccicheck" takes longer. What other downsides are >>> there? >> >> Once the global variable packed_git has been migrated out of >> existence, no new code that relies on it would be referring to that >> global variable. If coccicheck finds something, the suggested rewrite >> would be turning an unrelated packed_git (which may not even be the >> right type) to a reference to a field in a global variable, that >> would certainly be wrong. > > Ugh, yes. The semantic patch in question doesn't contain any type > information. I don't know how to match a variable by name *and* type. > Here's the closest I can come up with to a safe and complete > transformation, but it only handles assignments: > > @@ > struct packed_git *A; > identifier B = packed_git; > @@ > - A = B > + A = the_repository->objects.packed_git > I'll need to play more with coccinelle. :) Thanks for this patch. > Seeing the many for loops, I'm tempted to suggest adding a > for_each_packed_git macro to hide the global variable and thus reduce > the number of places to change at cutover. Coccinelle doesn't seem > to like them, though. > > A short semantic patch with a limited time of usefulness and possible > side-effects can easily be included in a commit message, of course.. In the shorter reroll I moved the semantic patch into a subdir of contrib/coccinelle, but in the next reroll I'll just put it into the commit message. Thanks, Stefan
