Web lists-archives.com

Re: [RFC PATCH] *.h: remove extern from function declarations




Jeff King <peff@xxxxxxxx> writes:

> So it sounds like you did that and the result is not _too_ bad. But I'm
> not sure about doing it automatically with sed. For example...
>
>> diff --git a/packfile.h b/packfile.h
>> index d70c6d9afb..dab50405e0 100644
>> --- a/packfile.h
>> +++ b/packfile.h
>> [...]
>> @@ -43,10 +43,10 @@ void for_each_file_in_pack_dir(const char *objdir,
>>  #define PACKDIR_FILE_PACK 1
>>  #define PACKDIR_FILE_IDX 2
>>  #define PACKDIR_FILE_GARBAGE 4
>> -extern void (*report_garbage)(unsigned seen_bits, const char *path);
>> +void (*report_garbage)(unsigned seen_bits, const char *path);
>
> This one is a function pointer, and so the extern is actually changing
> the visibility of the declared variable. It needs to stay.
>
> (I didn't read the whole patch carefully, but I knew to look for this
> one in particular since I had to deal with it in my patch, too).

Yeah, after reviewing your recent packfile patch, the above hunk was
what I went to hunt for directly in Denton's message ;-)

As I care much more about correctness than "conflicts with in-flight
topics too much", I'd prefer people to understand why we avoid the
wholesale update is because of an unnecessary bug like this one, not
the one-time conflict resolution load that can be subcontracted out
to "rerere" once dealt with ;-)

A wholesale rewrite using a tool more suited for the job (e.g.
spatch) is a different story, and raises the confidence level of the
end result a lot more than a "sed and then eyeball" rewrite, but any
tool that allows transformation that is sophisticated enough allows
bugs in the instructions we give to the tool, so...