Re: [PATCH v2 04/14] dir: select directories correctly

On 9/12/2021 6:21 PM, Ævar Arnfjörð Bjarmason wrote:

On Sun, Sep 12 2021, Derrick Stolee via GitGitGadget wrote:

quoted

+	/*
+	 * Use 'alloc' as an indicator that the string has not been
+	 * initialized, in case the parent is the root directory.
+	 */
+	if (!path_parent->alloc) {

This isn't wrong, but seems to be way too cozy with the internal
implementation details of strbuf. For what it's worth I renamed it to
"alloc2" and found that this would be only the 3rd bit of code out of
strbuf.[ch] that cares about that member.

I can understand not wanting to poke into the internals.

quoted

+		char *slash;
+		strbuf_addstr(path_parent, pathname);

So is "pathname" ever the empty string? If not we could check the
length?

We are given 'pathlen' as a parameter, so this should just use
strbuf_add() instead.

Or probably better: ...

quoted

@@ -1331,6 +1359,7 @@ static struct path_pattern *last_matching_pattern_from_list(const char *pathname
 {
 	struct path_pattern *res = NULL; /* undecided */
 	int i;
+	struct strbuf path_parent = STRBUF_INIT;

Just malloc + strbuf_init() this in the above function and have a
"struct strbuf *" initialized to NULL here? Then we can use a much more
idiomatic "is it NULL?" to check if it's initialized.

 
That makes sense. Can do.

Thanks,
-Stolee