From: Junio C Hamano <gitster@pobox.com> To: git@vger.kernel.org Subject: [PATCH 1/2] describe: do not use cmd_*() as a subroutine Date: Tue, 10 Oct 2017 13:06:03 +0900 Message-ID: <20171010040604.26029-2-gitster@pobox.com> (raw) In-Reply-To: <20171010040604.26029-1-gitster@pobox.com> The cmd_foo() function is a moral equivalent of 'main' for a Git subcommand 'git foo', and as such, it is allowed to do many things that make it unsuitable to be called as a subroutine, including - call exit(3) to terminate the process; - allocate resource held and used throughout its lifetime, without releasing it upon return/exit; - rely on global variables being initialized at program startup, and update them as needed, making another clean invocation of the function impossible. The call to cmd_diff_index() "git describe" makes has been working by accident that the function did not call exit(3); it sets a bad precedent for people to cut and paste. We could invoke it via the run_command() interface, but the diff family of commands have helper functions in diff-lib.c that are meant to be usable as subroutines, and using the latter does not make the resulting code all that longer. Use it. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- builtin/describe.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/builtin/describe.c b/builtin/describe.c index 89ea1cdd60..50263759cb 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -7,12 +7,12 @@ #include "builtin.h" #include "exec_cmd.h" #include "parse-options.h" +#include "revision.h" #include "diff.h" #include "hashmap.h" #include "argv-array.h" #include "run-command.h" -#define SEEN (1u << 0) #define MAX_TAGS (FLAG_BITS - 1) static const char * const describe_usage[] = { @@ -532,7 +532,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix) } } else if (dirty) { static struct lock_file index_lock; - int fd; + struct rev_info revs; + struct argv_array args = ARGV_ARRAY_INIT; + int fd, result; read_cache_preload(NULL); refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, @@ -541,8 +543,13 @@ int cmd_describe(int argc, const char **argv, const char *prefix) if (0 <= fd) update_index_if_able(&the_index, &index_lock); - if (!cmd_diff_index(ARRAY_SIZE(diff_index_args) - 1, - diff_index_args, prefix)) + init_revisions(&revs, prefix); + argv_array_pushv(&args, diff_index_args); + if (setup_revisions(args.argc, args.argv, &revs, NULL) != 1) + BUG("malformed internal diff-index command line"); + result = run_diff_index(&revs, 0); + + if (!diff_result_code(&revs.diffopt, result)) suffix = NULL; else suffix = dirty; -- 2.15.0-rc0-203-g4c8d0e28b1
next prev parent reply index Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-08-27 7:37 [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1() Martin Ågren 2017-08-27 15:41 ` Jeff King 2017-08-27 18:25 ` Jeff King 2017-08-27 18:21 ` Lars Schneider 2017-08-27 19:09 ` Martin Ågren 2017-08-27 19:15 ` Jeff King 2017-08-27 20:04 ` Lars Schneider 2017-08-27 23:23 ` Jeff King 2017-08-28 4:11 ` Martin Ågren 2017-08-28 17:52 ` Stefan Beller 2017-08-28 17:58 ` Jeff King 2017-09-05 10:03 ` Junio C Hamano 2017-08-29 17:51 ` Lars Schneider 2017-08-29 18:53 ` Jeff King 2017-08-29 18:58 ` [PATCH] config: use a static lock_file struct Jeff King 2017-08-29 19:09 ` Brandon Williams 2017-08-29 19:10 ` Brandon Williams 2017-08-29 19:12 ` Jeff King 2017-08-30 3:25 ` Michael Haggerty 2017-08-30 4:31 ` Jeff King 2017-08-30 4:55 ` Michael Haggerty 2017-08-30 4:55 ` Jeff King 2017-08-30 5:55 ` Jeff King 2017-08-30 7:07 ` Michael Haggerty 2017-09-02 8:44 ` Jeff King 2017-09-02 13:50 ` Jeff King 2017-08-30 6:55 ` Michael Haggerty 2017-08-30 19:53 ` Jeff King 2017-08-30 19:57 ` Brandon Williams 2017-08-30 20:11 ` Jeff King 2017-08-30 21:06 ` Brandon Williams 2017-08-31 4:09 ` Jeff King 2017-09-06 3:59 ` Junio C Hamano 2017-09-06 12:41 ` Jeff King 2017-08-29 19:22 ` [PATCH] pkt-line: re-'static'-ify buffer in packet_write_fmt_1() Martin Ågren 2017-08-29 21:48 ` Jeff King 2017-08-30 5:31 ` Jeff King 2017-09-05 10:03 ` Junio C Hamano 2017-10-10 4:06 ` [PATCH 0/2] Do not call cmd_*() as a subroutine Junio C Hamano 2017-10-10 4:06 ` Junio C Hamano [this message] 2017-10-10 13:43 ` [PATCH 1/2] describe: do not use " SZEDER Gábor 2017-10-11 6:00 ` Junio C Hamano 2017-10-10 4:06 ` [PATCH 2/2] merge-ours: " Junio C Hamano
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171010040604.26029-2-gitster@pobox.com \ --to=gitster@pobox.com \ --cc=git@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
git@vger.kernel.org mailing list mirror (one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox