Merge branch 'jk/snprintf-cleanups'
authorJunio C Hamano <gitster@pobox.com>
Mon, 17 Apr 2017 06:29:26 +0000 (23:29 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 17 Apr 2017 06:29:26 +0000 (23:29 -0700)
Code clean-up.

* jk/snprintf-cleanups:
daemon: use an argv_array to exec children
gc: replace local buffer with git_path
transport-helper: replace checked snprintf with xsnprintf
convert unchecked snprintf into xsnprintf
combine-diff: replace malloc/snprintf with xstrfmt
replace unchecked snprintf calls with heap buffers
receive-pack: print --pack-header directly into argv array
name-rev: replace static buffer with strbuf
create_branch: use xstrfmt for reflog message
create_branch: move msg setup closer to point of use
avoid using mksnpath for refs
avoid using fixed PATH_MAX buffers for refs
fetch: use heap buffer to format reflog
tag: use strbuf to format tag header
diff: avoid fixed-size buffer for patch-ids
odb_mkstemp: use git_path_buf
odb_mkstemp: write filename into strbuf
do not check odb_mkstemp return value for errors

1  2 
builtin/receive-pack.c
builtin/tag.c
cache.h
environment.c
submodule.c
diff --combined builtin/receive-pack.c
index aca9c33d8d867d7e507f10e2eae5617b23956cdf,2ca93adef576d787b765b6ee4200d301d30e0e0e..70202780819130ad56ea7391c319ed810d80b2a4
@@@ -1524,7 -1524,7 +1524,7 @@@ static void queue_commands_from_cert(st
  
        while (boc < eoc) {
                const char *eol = memchr(boc, '\n', eoc - boc);
 -              tail = queue_command(tail, boc, eol ? eol - boc : eoc - eol);
 +              tail = queue_command(tail, boc, eol ? eol - boc : eoc - boc);
                boc = eol ? eol + 1 : eoc;
        }
  }
@@@ -1634,12 -1634,17 +1634,17 @@@ static const char *parse_pack_header(st
  
  static const char *pack_lockfile;
  
+ static void push_header_arg(struct argv_array *args, struct pack_header *hdr)
+ {
+       argv_array_pushf(args, "--pack_header=%"PRIu32",%"PRIu32,
+                       ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
+ }
  static const char *unpack(int err_fd, struct shallow_info *si)
  {
        struct pack_header hdr;
        const char *hdr_err;
        int status;
-       char hdr_arg[38];
        struct child_process child = CHILD_PROCESS_INIT;
        int fsck_objects = (receive_fsck_objects >= 0
                            ? receive_fsck_objects
                        close(err_fd);
                return hdr_err;
        }
-       snprintf(hdr_arg, sizeof(hdr_arg),
-                       "--pack_header=%"PRIu32",%"PRIu32,
-                       ntohl(hdr.hdr_version), ntohl(hdr.hdr_entries));
  
        if (si->nr_ours || si->nr_theirs) {
                alt_shallow_file = setup_temporary_shallow(si->shallow);
        tmp_objdir_add_as_alternate(tmp_objdir);
  
        if (ntohl(hdr.hdr_entries) < unpack_limit) {
-               argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
+               argv_array_push(&child.args, "unpack-objects");
+               push_header_arg(&child.args, &hdr);
                if (quiet)
                        argv_array_push(&child.args, "-q");
                if (fsck_objects)
        } else {
                char hostname[256];
  
-               argv_array_pushl(&child.args, "index-pack",
-                                "--stdin", hdr_arg, NULL);
+               argv_array_pushl(&child.args, "index-pack", "--stdin", NULL);
+               push_header_arg(&child.args, &hdr);
  
                if (gethostname(hostname, sizeof(hostname)))
                        xsnprintf(hostname, sizeof(hostname), "localhost");
diff --combined builtin/tag.c
index dbc6f5b74b6c64153bc8e1cc5e24830586cbcfe1,0eb4e25c6b56adb096901b8ded032150b8d35855..222404522fd8db970c5f832338095c6947548ac9
@@@ -22,7 -22,7 +22,7 @@@
  static const char * const git_tag_usage[] = {
        N_("git tag [-a | -s | -u <key-id>] [-f] [-m <msg> | -F <file>] <tagname> [<head>]"),
        N_("git tag -d <tagname>..."),
 -      N_("git tag -l [-n[<num>]] [--contains <commit>] [--points-at <object>]"
 +      N_("git tag -l [-n[<num>]] [--contains <commit>] [--no-contains <commit>] [--points-at <object>]"
                "\n\t\t[--format=<format>] [--[no-]merged [<commit>]] [<pattern>...]"),
        N_("git tag -v [--format=<format>] <tagname>..."),
        NULL
@@@ -72,25 -72,22 +72,22 @@@ static int for_each_tag_name(const cha
                             const void *cb_data)
  {
        const char **p;
-       char ref[PATH_MAX];
+       struct strbuf ref = STRBUF_INIT;
        int had_error = 0;
        unsigned char sha1[20];
  
        for (p = argv; *p; p++) {
-               if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
-                                       >= sizeof(ref)) {
-                       error(_("tag name too long: %.*s..."), 50, *p);
-                       had_error = 1;
-                       continue;
-               }
-               if (read_ref(ref, sha1)) {
+               strbuf_reset(&ref);
+               strbuf_addf(&ref, "refs/tags/%s", *p);
+               if (read_ref(ref.buf, sha1)) {
                        error(_("tag '%s' not found."), *p);
                        had_error = 1;
                        continue;
                }
-               if (fn(*p, ref, sha1, cb_data))
+               if (fn(*p, ref.buf, sha1, cb_data))
                        had_error = 1;
        }
+       strbuf_release(&ref);
        return had_error;
  }
  
@@@ -231,26 -228,22 +228,22 @@@ static void create_tag(const unsigned c
                       unsigned char *prev, unsigned char *result)
  {
        enum object_type type;
-       char header_buf[1024];
-       int header_len;
+       struct strbuf header = STRBUF_INIT;
        char *path = NULL;
  
        type = sha1_object_info(object, NULL);
        if (type <= OBJ_NONE)
            die(_("bad object type."));
  
-       header_len = snprintf(header_buf, sizeof(header_buf),
-                         "object %s\n"
-                         "type %s\n"
-                         "tag %s\n"
-                         "tagger %s\n\n",
-                         sha1_to_hex(object),
-                         typename(type),
-                         tag,
-                         git_committer_info(IDENT_STRICT));
-       if (header_len > sizeof(header_buf) - 1)
-               die(_("tag header too big."));
+       strbuf_addf(&header,
+                   "object %s\n"
+                   "type %s\n"
+                   "tag %s\n"
+                   "tagger %s\n\n",
+                   sha1_to_hex(object),
+                   typename(type),
+                   tag,
+                   git_committer_info(IDENT_STRICT));
  
        if (!opt->message_given) {
                int fd;
        if (!opt->message_given && !buf->len)
                die(_("no tag message?"));
  
-       strbuf_insert(buf, 0, header_buf, header_len);
+       strbuf_insert(buf, 0, header.buf, header.len);
+       strbuf_release(&header);
  
        if (build_tag_object(buf, opt->sign, result) < 0) {
                if (path)
@@@ -424,17 -418,14 +418,17 @@@ int cmd_tag(int argc, const char **argv
                OPT_GROUP(N_("Tag listing options")),
                OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
                OPT_CONTAINS(&filter.with_commit, N_("print only tags that contain the commit")),
 +              OPT_NO_CONTAINS(&filter.no_commit, N_("print only tags that don't contain the commit")),
                OPT_WITH(&filter.with_commit, N_("print only tags that contain the commit")),
 +              OPT_WITHOUT(&filter.no_commit, N_("print only tags that don't contain the commit")),
                OPT_MERGED(&filter, N_("print only tags that are merged")),
                OPT_NO_MERGED(&filter, N_("print only tags that are not merged")),
                OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
                             N_("field name to sort on"), &parse_opt_ref_sorting),
                {
                        OPTION_CALLBACK, 0, "points-at", &filter.points_at, N_("object"),
 -                      N_("print only tags of the object"), 0, parse_opt_object_name
 +                      N_("print only tags of the object"), PARSE_OPT_LASTARG_DEFAULT,
 +                      parse_opt_object_name, (intptr_t) "HEAD"
                },
                OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
                OPT_BOOL('i', "ignore-case", &icase, N_("sorting and filtering are case insensitive")),
        }
        create_tag_object = (opt.sign || annotate || msg.given || msgfile);
  
 -      if (argc == 0 && !cmdmode)
 -              cmdmode = 'l';
 +      if (!cmdmode) {
 +              if (argc == 0)
 +                      cmdmode = 'l';
 +              else if (filter.with_commit || filter.no_commit ||
 +                       filter.points_at.nr || filter.merge_commit ||
 +                       filter.lines != -1)
 +                      cmdmode = 'l';
 +      }
  
        if ((create_tag_object || force) && (cmdmode != 0))
                usage_with_options(git_tag_usage, options);
                return ret;
        }
        if (filter.lines != -1)
 -              die(_("-n option is only allowed with -l."));
 +              die(_("-n option is only allowed in list mode"));
        if (filter.with_commit)
 -              die(_("--contains option is only allowed with -l."));
 +              die(_("--contains option is only allowed in list mode"));
 +      if (filter.no_commit)
 +              die(_("--no-contains option is only allowed in list mode"));
        if (filter.points_at.nr)
 -              die(_("--points-at option is only allowed with -l."));
 +              die(_("--points-at option is only allowed in list mode"));
        if (filter.merge_commit)
 -              die(_("--merged and --no-merged option are only allowed with -l"));
 +              die(_("--merged and --no-merged options are only allowed in list mode"));
        if (cmdmode == 'd')
                return for_each_tag_name(argv, delete_tag, NULL);
        if (cmdmode == 'v') {
diff --combined cache.h
index 5c8078291c478f579d6b8bcc69417ac6adb1ced0,4f3bdd0b5d4f29cb3d263519a7303acd2b99cc2a..556468c25b8c3a06eb776fbf322bed805ec97c33
+++ b/cache.h
@@@ -411,7 -411,6 +411,7 @@@ static inline enum object_type object_t
  #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE"
  #define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX"
  #define GIT_SUPER_PREFIX_ENVIRONMENT "GIT_INTERNAL_SUPER_PREFIX"
 +#define GIT_TOPLEVEL_PREFIX_ENVIRONMENT "GIT_INTERNAL_TOPLEVEL_PREFIX"
  #define DEFAULT_GIT_DIR_ENVIRONMENT ".git"
  #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY"
  #define INDEX_ENVIRONMENT "GIT_INDEX_FILE"
@@@ -1675,9 -1674,12 +1675,12 @@@ extern struct packed_git *find_sha1_pac
  extern void pack_report(void);
  
  /*
-  * Create a temporary file rooted in the object database directory.
+  * Create a temporary file rooted in the object database directory, or
+  * die on failure. The filename is taken from "pattern", which should have the
+  * usual "XXXXXX" trailer, and the resulting filename is written into the
+  * "template" buffer. Returns the open descriptor.
   */
- extern int odb_mkstemp(char *template, size_t limit, const char *pattern);
+ extern int odb_mkstemp(struct strbuf *template, const char *pattern);
  
  /*
   * Generate the filename to be used for a pack file with checksum "sha1" and
diff --combined environment.c
index a17d96aa85bd9196da9090c768090aeec8b6b8be,a9bf5658ade92436feb96f68f174a484fd09fd32..ff6e4f06e93d642aa53ba1812c3788b0ccad092e
@@@ -167,11 -167,8 +167,11 @@@ static void setup_git_env(void
        const char *replace_ref_base;
  
        git_dir = getenv(GIT_DIR_ENVIRONMENT);
 -      if (!git_dir)
 +      if (!git_dir) {
 +              if (!startup_info->have_repository)
 +                      die("BUG: setup_git_env called without repository");
                git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
 +      }
        gitfile = read_gitfile(git_dir);
        git_dir = xstrdup(gitfile ? gitfile : git_dir);
        if (get_common_dir(&sb, git_dir))
@@@ -277,7 -274,7 +277,7 @@@ char *get_object_directory(void
        return git_object_dir;
  }
  
- int odb_mkstemp(char *template, size_t limit, const char *pattern)
+ int odb_mkstemp(struct strbuf *template, const char *pattern)
  {
        int fd;
        /*
         * restrictive except to remove write permission.
         */
        int mode = 0444;
-       snprintf(template, limit, "%s/%s",
-                get_object_directory(), pattern);
-       fd = git_mkstemp_mode(template, mode);
+       git_path_buf(template, "objects/%s", pattern);
+       fd = git_mkstemp_mode(template->buf, mode);
        if (0 <= fd)
                return fd;
  
        /* slow path */
        /* some mkstemp implementations erase template on failure */
-       snprintf(template, limit, "%s/%s",
-                get_object_directory(), pattern);
-       safe_create_leading_directories(template);
-       return xmkstemp_mode(template, mode);
+       git_path_buf(template, "objects/%s", pattern);
+       safe_create_leading_directories(template->buf);
+       return xmkstemp_mode(template->buf, mode);
  }
  
  int odb_pack_keep(const char *name)
diff --combined submodule.c
index c52d6634c528d79ddedf069b7e09e955d283ed64,c9f39ebbe55ecfe88f26f43f83b01bd16c7c4962..83b13b8ff5fa31a213dcd1d87f67a919441494fa
@@@ -213,59 -213,25 +213,59 @@@ void gitmodules_config_sha1(const unsig
  }
  
  /*
 + * NEEDSWORK: With the addition of different configuration options to determine
 + * if a submodule is of interests, the validity of this function's name comes
 + * into question.  Once the dust has settled and more concrete terminology is
 + * decided upon, come up with a more proper name for this function.  One
 + * potential candidate could be 'is_submodule_active()'.
 + *
   * Determine if a submodule has been initialized at a given 'path'
   */
  int is_submodule_initialized(const char *path)
  {
        int ret = 0;
 -      const struct submodule *module = NULL;
 +      char *key = NULL;
 +      char *value = NULL;
 +      const struct string_list *sl;
 +      const struct submodule *module = submodule_from_path(null_sha1, path);
  
 -      module = submodule_from_path(null_sha1, path);
 +      /* early return if there isn't a path->module mapping */
 +      if (!module)
 +              return 0;
  
 -      if (module) {
 -              char *key = xstrfmt("submodule.%s.url", module->name);
 -              char *value = NULL;
 +      /* submodule.<name>.active is set */
 +      key = xstrfmt("submodule.%s.active", module->name);
 +      if (!git_config_get_bool(key, &ret)) {
 +              free(key);
 +              return ret;
 +      }
 +      free(key);
  
 -              ret = !git_config_get_string(key, &value);
 +      /* submodule.active is set */
 +      sl = git_config_get_value_multi("submodule.active");
 +      if (sl) {
 +              struct pathspec ps;
 +              struct argv_array args = ARGV_ARRAY_INIT;
 +              const struct string_list_item *item;
  
 -              free(value);
 -              free(key);
 +              for_each_string_list_item(item, sl) {
 +                      argv_array_push(&args, item->string);
 +              }
 +
 +              parse_pathspec(&ps, 0, 0, NULL, args.argv);
 +              ret = match_pathspec(&ps, path, strlen(path), 0, NULL, 1);
 +
 +              argv_array_clear(&args);
 +              clear_pathspec(&ps);
 +              return ret;
        }
  
 +      /* fallback to checking if the URL is set */
 +      key = xstrfmt("submodule.%s.url", module->name);
 +      ret = !git_config_get_string(key, &value);
 +
 +      free(value);
 +      free(key);
        return ret;
  }
  
@@@ -1436,7 -1402,7 +1436,7 @@@ static int find_first_merges(struct obj
        memset(&rev_opts, 0, sizeof(rev_opts));
  
        /* get all revisions that merge commit a */
-       snprintf(merged_revision, sizeof(merged_revision), "^%s",
+       xsnprintf(merged_revision, sizeof(merged_revision), "^%s",
                        oid_to_hex(&a->object.oid));
        init_revisions(&revs, NULL);
        rev_opts.submodule = path;