Hi Shourya, On Thu, 16 Jul 2020, Shourya Shukla wrote: > BTW in your review of my patch, > https://lore.kernel.org/git/nycvar.QRO.7.76.6.2007031712160.50@xxxxxxxxxxxxxxxxx/ > you said this: > --------------->8-------------- > > + */ > > + struct child_process cp_rev_parse = CHILD_PROCESS_INIT; > > + struct strbuf sb_rev_parse = STRBUF_INIT; > > + > > + cp_rev_parse.git_cmd = 1; > > + cp_rev_parse.no_stderr = 1; > > + cp_rev_parse.dir = p->sm_path; > > + prepare_submodule_repo_env(&cp_rev_parse.env_array); > > + > > + argv_array_pushl(&cp_rev_parse.args, "rev-parse", > > + "HEAD", NULL); > > + if (!capture_command(&cp_rev_parse, &sb_rev_parse, 0)) { > > + strbuf_strip_suffix(&sb_rev_parse, "\n"); > > + get_oid_hex(sb_rev_parse.buf, &p->oid_dst); > > + } > > + strbuf_release(&sb_rev_parse); > > + } else if (S_ISLNK(p->mod_dst) || S_ISREG(p->mod_dst)) { > > + struct child_process cp_hash_object = CHILD_PROCESS_INIT; > > + struct strbuf sb_hash_object = STRBUF_INIT; > > + > > + cp_hash_object.git_cmd = 1; > > + argv_array_pushl(&cp_hash_object.args, "hash-object", > > + p->sm_path, NULL); > > + if (!capture_command(&cp_hash_object, > > + &sb_hash_object, 0)) { > > + strbuf_strip_suffix(&sb_hash_object, "\n"); > > + get_oid_hex(sb_hash_object.buf, &p->oid_dst); > > + } > > + strbuf_release(&sb_hash_object); > > It would probably be shorter, less error-prone, and quicker to use > `index_fd()` directly. > --------------->8-------------- > > What exactly did you mean here and where should the index_fd() be used? Well, `index_fd()` is the function that `git hash-object` uses to calculate the hash of a given file (in this case, specified via the path `p->sm_path`). You could open an fd to that file directly, use `index_fd()` to calculate the hash, and thereby avoid spawning another (totally unnecessary) process. Ciao, Dscho
