On Wed, Jun 18, 2014 at 08:52:46PM +0100, Ramsay Jones wrote:
> In order to encapsulate the setting of the unique commit index, commit
> 969eba63 ("commit: push commit_index update into alloc_commit_node",
> 10-06-2014) introduced a (logically private) intermediary allocator
> function. However, this function (alloc_raw_commit_node()) was declared
> as a public function, which undermines its entire purpose.
>
> Remove the alloc_raw_commit_node() function and inline its code into
> the (public) alloc_commit_node() function.
>
> Noticed by sparse ("symbol 'alloc_raw_commit_node' was not declared.
> Should it be static?").
>
> Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx>
> ---
>
> Hi Jeff,
>
> I noticed this while it was still in 'pu', but got distracted and
> didn't send this in time ... sorry about that! :(
Yeah, I noticed it while writing the patch but decided it wasn't worth
the trouble to deal with (since after all, it's not advertised to any
callers, the very thing that sparse is complaining about. :) ).
I don't mind fixing it, though I really don't like repeating the
contents of DEFINE_ALLOCATOR. I know it hasn't changed in a while, but
it just feels wrong.
> My first attempt at fixing this involved changing the DEFINE_ALLOCATOR
> macro to include a 'scope' parameter so that I could declare the
> raw_commit allocator 'static'. Unfortunately, I could not pass the
> extern keyword as the scope parameter to all the other allocators,
> because that made sparse even more upset - you can't use extern on
> a function _definition_. That meant passing an empty argument (or a
> comment token) to the scope parameter. This worked for gcc 4.8.2 and
> clang 3.4, but I was a little concerned about portability.
Yeah, passing an empty argument was my first thought, but I don't know
offhand if there are portability concerns.
You could also have DEFINE_ALLOCATOR just fill in the body, and do:
struct blob *alloc_blob_node(void)
{
DEFINE_ALLOCATOR(struct blob);
}
or similar.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html