Re: [PATCH 1/2] submodule--helper: remove an unreachable call to usage_with_options
- From
Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
- Date
- Jun 22, 2021, 18:02 UTC
- Message-ID
- <7c1522ab-26b8-66b2-46e2-7b974c763b3b@gmail.com>
- In-Reply-To
- <CAPig+cT66BT7fCfHBJM25D1SBVKAwRpSh+SAaR8YmqZX+7epvA@mail.gmail.com>
On 22/06/21 1:28 am, Eric Sunshine wrote:
Show 12 quoted lines
> 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.
Show 14 quoted lines
>> 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