Re: [PATCH 01/11] maintenance: create basic maintenance runner

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

 



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?

>> +
>> +	memset(&opts, 0, sizeof(opts));
>
> not needed if it's static.  If it's not static, could use `opts = {0}`
> where it's declared to do the same thing in a simpler way.

Okay, so that's one upside, albeit a small one.




[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