diff mbox series

[1/2] submodule--helper: remove an unreachable call to usage_with_options

Message ID 20210621190837.9487-2-kaartic.sivaraam@gmail.com (mailing list archive)
State Superseded
Headers show
Series Some submodule related code cleanup | expand

Commit Message

Kaartic Sivaraam June 21, 2021, 7:08 p.m. UTC
The code path in question calls `error` in a particular case.
But, `error` never returns as it exits directly. This makes
the call to `usage_with_options` that follows the `error` call
unreachable.

So, remove the unreachable `usage_with_options` call.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 builtin/submodule--helper.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Eric Sunshine June 21, 2021, 7:58 p.m. UTC | #1
On Mon, Jun 21, 2021 at 3:09 PM Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
> The code path in question calls `error` in a particular case.
> But, `error` never returns as it exits directly. This makes
> the call to `usage_with_options` that follows the `error` call
> unreachable.

error() returns -1; you will commonly see:

    if (check_something())
        return error(...);

> So, remove the unreachable `usage_with_options` call.
>
> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> @@ -1637,8 +1637,6 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
>         if (all && argc) {
>                 error("pathspec and --all are incompatible");
> -               usage_with_options(git_submodule_helper_usage,
> -                                  module_deinit_options);
>         }

usage_with_options(), on the other hand, exits directly.
Kaartic Sivaraam June 22, 2021, 6:02 p.m. UTC | #2
On 22/06/21 1:28 am, Eric Sunshine wrote:
> On Mon, Jun 21, 2021 at 3:09 PM Kaartic Sivaraam
> <kaartic.sivaraam@gmail.com> wrote:
>> The code path in question calls `error` in a particular case.
>> But, `error` never returns as it exits directly. This makes
>> the call to `usage_with_options` that follows the `error` call
>> unreachable.
> 
> error() returns -1; you will commonly see:
> 
>      if (check_something())
>          return error(...);
>

You're right. I guess I was drowsy when I was looking at this
part for the code. The passing tests didn't help either.

>> So, remove the unreachable `usage_with_options` call.
>>
>> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
>> ---
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> @@ -1637,8 +1637,6 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
>>          if (all && argc) {
>>                  error("pathspec and --all are incompatible");
>> -               usage_with_options(git_submodule_helper_usage,
>> -                                  module_deinit_options);
>>          }
> 
> usage_with_options(), on the other hand, exits directly.
> 

Got it. Will drop this patch and re-roll.

Thanks,
Sivaraam
diff mbox series

Patch

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ae6174ab05..c9aa838083 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1637,8 +1637,6 @@  static int module_deinit(int argc, const char **argv, const char *prefix)
 
 	if (all && argc) {
 		error("pathspec and --all are incompatible");
-		usage_with_options(git_submodule_helper_usage,
-				   module_deinit_options);
 	}
 
 	if (!argc && !all)