Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
>>> +static int maintenance_run(struct maintenance_opts *opts)
>>> +{
>>> + return maintenance_task_gc(opts);
>>> +}
>>> +
>>> +int cmd_maintenance(int argc, const char **argv, const char *prefix)
>>> +{
>>> + static struct maintenance_opts opts;
>>> + static struct option builtin_maintenance_options[] = {
>>> + OPT_BOOL(0, "auto", &opts.auto_flag,
>>> + N_("run tasks based on the state of the repository")),
>>> + OPT_END()
>>> + };
>>
>> optional: these could be local instead of static
>
> Do we have preference? I think it is more common in our codebase to
> have these non-static but I do not think we've chosen which is more
> preferable with technical reasoning behind it. As the top-level
> function in any callchain, cmd_foo() does not have multiple instances
> running at the same time, or it does not have to be prepared to be
> called twice, so they can afford to be static, but is there any upside
> for them to be static?
Code size, execution time:
- benefit of static: can rely on automatic zero-initialization of
pages instead of spending cycles and text size on explicit
zero-initialization
- benefit of local: avoids wasting address space in bss when the
function isn't called
Neither of those seems important enough to care about. :)
Maintainability:
- benefit of static: address is determined at compile time, so can build
using C89 compilers that require struct initializers to be constant
(but Git already cannot be built with such compilers)
- benefit of local: less likely to accidentally move the static var into
a function that needs to be reentrant
- benefit of local: allows readers and reviewers to build the habit of
seeing non-const "static" declarations within a function as the
unusual case. When a "static" is declared in a function body, this
means we are caching something or have some other performance reason
to sacrifice reentrancy
It's mostly that last aspect (saving readers from having to think
about it) that motivates my suggestion above.
Older Git code used static more because it was needed when building
using C89 compilers.
Thanks,
Jonathan
