From: Junio C Hamano Date: Sun, 24 May 2020 23:18:36 GMT Subject: Re: [PATCH v3] submodule: port subcommand 'set-branch' from shell to C Message-ID: In-Reply-To: Junio C Hamano writes: > Johannes Schindelin writes: > >>> + config_set_in_gitmodules_file_gently(config_name, opt_branch); >> >> What happens if this fails? E.g. when the permission is denied or disk is >> full? This C code would then still `return 0`, pretending that it >> succeeded. But the original shell script calls `git submodule--helper >> config [...]` which calls `module_config()`, which in turn passes through >> the return value of the `config_set_in_gitmodules_file_gently()` call. >> >> In other words, you need something like this: >> >> int ret; >> >> [...] >> >> ret = config_set_in_gitmodules_file_gently(config_name, opt_branch); >> >> free(config_name); >> return ret; > > Making sure we check the return value of helper functions we call is > a good discipline,... By the way, another topic by you for set-url has exactly the same issue. Its call to config_set_in_gitmodules_file_gently() can fail. So can the call to sync_submodule(), but when it fails it won't come back, so we do not have to worry about not capturing its return value ;-)