# [PATCH] pack-objects: use strcspn(3) in name_cmp_len() 3 messages from 2023-02-05 to 2023-02-06. Participants: René Scharfe, Ævar Arnfjörð Bjarmason, Junio C Hamano. Thread: https://gitlist.dev/t/59199 ## René Scharfe, 2023-02-05 10:42 Subject: [PATCH] pack-objects: use strcspn(3) in name_cmp_len() Message-ID: <7315487c-c97c-b8a2-d3b2-4fbf642495dd@web.de> URL: https://gitlist.dev/e/7315487c-c97c-b8a2-d3b2-4fbf642495dd%40web.de ``` Call strcspn(3) to find the length of a string terminated by NUL, NL or slash instead of open-coding it. Adopt its return type, size_t, to support strings of arbitrary length. Use that type in callers as well for variables and function parameters that receive the return value. Signed-off-by: René Scharfe --- Formatted with --function-context for easier review. builtin/pack-objects.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index cabace4abb..536edc3a48 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1708,50 +1708,47 @@ static void pbase_tree_put(struct pbase_tree_cache *cache) free(cache); } -static int name_cmp_len(const char *name) +static size_t name_cmp_len(const char *name) { - int i; - for (i = 0; name[i] && name[i] != '\n' && name[i] != '/'; i++) - ; - return i; + return strcspn(name, "\n/"); } static void add_pbase_object(struct tree_desc *tree, const char *name, - int cmplen, + size_t cmplen, const char *fullname) { struct name_entry entry; int cmp; while (tree_entry(tree,&entry)) { if (S_ISGITLINK(entry.mode)) continue; cmp = tree_entry_len(&entry) != cmplen ? 1 : memcmp(name, entry.path, cmplen); if (cmp > 0) continue; if (cmp < 0) return; if (name[cmplen] != '/') { add_object_entry(&entry.oid, object_type(entry.mode), fullname, 1); return; } if (S_ISDIR(entry.mode)) { struct tree_desc sub; struct pbase_tree_cache *tree; const char *down = name+cmplen+1; - int downlen = name_cmp_len(down); + size_t downlen = name_cmp_len(down); tree = pbase_tree_get(&entry.oid); if (!tree) return; init_tree_desc(&sub, tree->tree_data, tree->tree_size); add_pbase_object(&sub, down, downlen, fullname); pbase_tree_put(tree); } } } @@ -1795,21 +1792,21 @@ static int check_pbase_path(unsigned hash) static void add_preferred_base_object(const char *name) { struct pbase_tree *it; - int cmplen; + size_t cmplen; unsigned hash = pack_name_hash(name); if (!num_preferred_base || check_pbase_path(hash)) return; cmplen = name_cmp_len(name); for (it = pbase_tree; it; it = it->next) { if (cmplen == 0) { add_object_entry(&it->pcache.oid, OBJ_TREE, NULL, 1); } else { struct tree_desc tree; init_tree_desc(&tree, it->pcache.tree_data, it->pcache.tree_size); add_pbase_object(&tree, name, cmplen, name); } } } -- 2.39.1 ``` ## Ævar Arnfjörð Bjarmason, 2023-02-05 20:59 Subject: Re: [PATCH] pack-objects: use strcspn(3) in name_cmp_len() Message-ID: <230205.86357j95wj.gmgdl@evledraar.gmail.com> URL: https://gitlist.dev/e/230205.86357j95wj.gmgdl%40evledraar.gmail.com In-Reply-To: <7315487c-c97c-b8a2-d3b2-4fbf642495dd@web.de> ``` On Sun, Feb 05 2023, René Scharfe wrote: > -static int name_cmp_len(const char *name) > +static size_t name_cmp_len(const char *name) > { > - int i; > - for (i = 0; name[i] && name[i] != '\n' && name[i] != '/'; i++) > - ; > - return i; > + return strcspn(name, "\n/"); > } I wonder if this name_cmp_len() is worth keeping at all. If all we're doing is wrapping strcspn() (which b.t.w, seem to be less "open-coding" and just that it wasn't known to the original author in 5d4a6003354 (Make git-pack-objects a builtin, 2006-08-03)), then just inlining that in the two name_cmp_len() invocations would be better, or maybe: strcspn(..., object_reject); Where we previously declared "object_reject" (or same better name for such a variable) to be "\n/". > static void add_pbase_object(struct tree_desc *tree, > const char *name, > - int cmplen, > + size_t cmplen, > const char *fullname) > { > struct name_entry entry; > int cmp; > > while (tree_entry(tree,&entry)) { > if (S_ISGITLINK(entry.mode)) > continue; > cmp = tree_entry_len(&entry) != cmplen ? 1 : > memcmp(name, entry.path, cmplen); The return of tree_entry_len() is "int", so that's a new implicit cast, but that seems OK in this case (and the "namelen" is another thing we should convert to "size_t"). ``` ## Junio C Hamano, 2023-02-06 22:35 Subject: Re: [PATCH] pack-objects: use strcspn(3) in name_cmp_len() Message-ID: URL: https://gitlist.dev/e/xmqqr0v22zio.fsf%40gitster.g In-Reply-To: <230205.86357j95wj.gmgdl@evledraar.gmail.com> ``` Ævar Arnfjörð Bjarmason writes: > I wonder if this name_cmp_len() is worth keeping at all. If all we're > doing is wrapping strcspn() (which b.t.w, seem to be less "open-coding" > and just that it wasn't known to the original author in 5d4a6003354 > (Make git-pack-objects a builtin, 2006-08-03)), then just inlining that > in the two name_cmp_len() invocations would be better, or maybe: Even if the stop candidate bytes were a constant, or if there were only a single callsite, I am not sure if it is a good idea, simply because with this > strcspn(..., object_reject); or with a literal "\n/" to make it easier to see where in the string we are stopping, it is hard without named function to tell what length we are computing. The function being file-scope static, decent compilers hopefully would inline the calls by two callers. ```