Re: [PATCH 003/194] object-store: move packed_git and packed_git_mru to object store

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux