git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

Previous: Eric SunshineNext: Kaartic Sivaraam
Message 4 of 7 in “Some submodule related code cleanup”
  1. 0/2 Some submodule related code cleanupKaartic Sivaraam, Jun 21, 2021
  2. 1/2 submodule--helper: remove an unreachable call to usage_with_optionsKaartic Sivaraam, Jun 21, 2021
  3. Eric SunshineJun 21, 2021
  4. Kaartic SivaraamJun 22, 2021
  5. 2/2 submodule: remove unnecessary `prefix` based option logicKaartic Sivaraam, Jun 21, 2021
  6. 0/1 Some submodule related code cleanupKaartic Sivaraam, Jun 22, 2021
  7. 1/1 submodule: remove unnecessary `prefix` based option logicKaartic Sivaraam, Jun 22, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.