Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> Use the less verbose { 0 }-initialization syntax rather than memset()
> in builtin/submodule--helper.c, this doesn't make a difference in
> terms of behavior, but as we're about to modify adjacent code makes
> this more consistent, and lets us avoid worrying about when the
> memset() happens v.s. a "goto cleanup".
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
> builtin/submodule--helper.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
These four hunks form two pairs of two hunks, one declaring (and in
the new code, initializing) a variable, the other clearing (and in
the new code, there is no need to) the variable.
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index fac52ade5e1..73717be957c 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -1744,7 +1744,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> {
> int dissociate = 0, quiet = 0, progress = 0, require_init = 0;
> struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;
> - struct list_objects_filter_options filter_options;
> + struct list_objects_filter_options filter_options = { 0 };
>
> struct option module_clone_options[] = {
> OPT_STRING(0, "prefix", &clone_data.prefix,
> @@ -1786,7 +1786,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> NULL
> };
>
> - memset(&filter_options, 0, sizeof(filter_options));
> argc = parse_options(argc, argv, prefix, module_clone_options,
> git_submodule_helper_usage, 0);
> @@ -2563,7 +2562,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
> {
> struct pathspec pathspec;
> struct update_data opt = UPDATE_DATA_INIT;
> - struct list_objects_filter_options filter_options;
> + struct list_objects_filter_options filter_options = { 0 };
> int ret;
>
> struct option module_update_options[] = {
> @@ -2623,7 +2622,6 @@ static int module_update(int argc, const char **argv, const char *prefix)
> update_clone_config_from_gitmodules(&opt.max_jobs);
> git_config(git_update_clone_config, &opt.max_jobs);
>
> - memset(&filter_options, 0, sizeof(filter_options));
> argc = parse_options(argc, argv, prefix, module_update_options,
> git_submodule_helper_usage, 0);
The fact that this patch is harder to read without wider context
tells us that these two changes are good idea. Absolutely nothing
happens in the first function between the declaration of the
variable and memset() but it is hard to see in a patch because they
are so far apart. In the second function, some things happen but
they are not using the variable before they get initialized, but
again, it is hard to see in a patch without wider context.
The post-image of this patch allows readers easily to see that
nobody uses these variables without initialization, which is a great
improvement.
Thanks, will queue.
