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