threads / patch / 63663

patchportability: allow building in systems without d_type

Subject: [PATCH] portability: allow building in systems without d_type

## tl;dr

6 messages between Jun 18, 2025 and Jun 18, 2025. Diffs are folded; open one to read it.

replies: 5people: 5as markdown or json

Carlo Marcelo Arenas Belón· Jun 18, 2025, 06:23 UTC · lore

Since 09fb155f11 (diff --no-index: support limiting by pathspec, 2025-05-21) will fail to build in platforms that don't have a d_type member on their struct dirent (ex: AIX, NonStop).

Use the DTYPE() macro instead of a nake reference to d_type.
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 diff-no-index.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to diff-no-index.c +1 −1
diff --git a/diff-no-index.c b/diff-no-index.c
index 4aeeb98cfa..7c95222ba6 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list,
 
 			if (!match_leading_pathspec(NULL, pathspec,
 						    match.buf, match.len,
-						    0, NULL, e->d_type == DT_DIR ? 1 : 0))
+						    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))
 				continue;
 		}
 
-- 
2.50.0.53.g63c9ac04f7
Collin Funk· Jun 18, 2025, 06:39 UTC · re: Carlo Marcelo Arenas Belón · lore

Re: [PATCH] portability: allow building in systems without d_type

Hi Carlo,
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 23 quoted lines
> Since 09fb155f11 (diff --no-index: support limiting by pathspec,
> 2025-05-21) will fail to build in platforms that don't have a
> d_type member on their struct dirent (ex: AIX, NonStop).
>
> Use the DTYPE() macro instead of a nake reference to d_type.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>  diff-no-index.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 4aeeb98cfa..7c95222ba6 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list,
>  
>  			if (!match_leading_pathspec(NULL, pathspec,
>  						    match.buf, match.len,
> -						    0, NULL, e->d_type == DT_DIR ? 1 : 0))
> +						    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))
>  				continue;
>  		}

I confirm that before this patch the build will fail with the following on AIX 7.3:

        CC diff-no-index.o
    diff-no-index.c: In function 'read_directory_contents':
    diff-no-index.c:49:21: error: 'struct dirent' has no member named 'd_type'
       49 |           0, NULL, e->d_type == DT_DIR ? 1 : 0))
          |                     ^~
    gmake: *** [Makefile:2821: diff-no-index.o] Error 1
This patch fixes it. Thanks.
Reviewed-by: Collin Funk <collin.funk1@gmail.com>
Collin
Marc Branchaud· Jun 18, 2025, 14:12 UTC · re: Carlo Marcelo Arenas Belón · lore

Re: [PATCH] portability: allow building in systems without d_type

On 2025-06-18 02:23, Carlo Marcelo Arenas Belón wrote:
> Since 09fb155f11 (diff --no-index: support limiting by pathspec,
> 2025-05-21) will fail to build in platforms that don't have a
s/will fail/git fails/
> d_type member on their struct dirent (ex: AIX, NonStop).
> 
> Use the DTYPE() macro instead of a nake reference to d_type.
s/nake/naked/
		M.
Show 18 quoted lines
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
>   diff-no-index.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 4aeeb98cfa..7c95222ba6 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -46,7 +46,7 @@ static int read_directory_contents(const char *path, struct string_list *list,
>   
>   			if (!match_leading_pathspec(NULL, pathspec,
>   						    match.buf, match.len,
> -						    0, NULL, e->d_type == DT_DIR ? 1 : 0))
> +						    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))
>   				continue;
>   		}
>   
Kristoffer Haugsbakk· Jun 18, 2025, 14:32 UTC · re: Carlo Marcelo Arenas Belón · lore

Re: [PATCH] portability: allow building in systems without d_type

On Wed, Jun 18, 2025, at 08:23, Carlo Marcelo Arenas Belón wrote:
> Since 09fb155f11 (diff --no-index: support limiting by pathspec,
> 2025-05-21) will fail to build in platforms that don't have a
> d_type member on their struct dirent (ex: AIX, NonStop).
s/build in/build on/
-- 
Kristoffer Haugsbakk
Junio C Hamano· Jun 18, 2025, 18:20 UTC · re: Carlo Marcelo Arenas Belón · lore

Re: [PATCH] portability: allow building in systems without d_type

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
Show 5 quoted lines
> Since 09fb155f11 (diff --no-index: support limiting by pathspec,
> 2025-05-21) will fail to build in platforms that don't have a
> d_type member on their struct dirent (ex: AIX, NonStop).
>
> Use the DTYPE() macro instead of a nake reference to d_type.

This may allow you to compile and build, but does the resulting binary do what you want it to?

>  			if (!match_leading_pathspec(NULL, pathspec,
>  						    match.buf, match.len,
> -						    0, NULL, e->d_type == DT_DIR ? 1 : 0))
> +						    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))

On a platform without d_type member, DTYPE() macro gives DT_UNKNOWN that is not DT_DIR, so essentially you are always passing 0 even when you are looking at a directory (in which case you must pass 1) to match_leading_pathspec().

So I somehow doubt this is a correct fix.

I do not know if get_dtype() helper function is easily applicable to this codepath, so I wrote this in a longhand...

 diff-no-index.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
Show changes to diff +17 −1
diff --git c/diff-no-index.c w/diff-no-index.c
index 7c95222ba6..677df91fc5 100644
--- c/diff-no-index.c
+++ w/diff-no-index.c
@@ -41,12 +41,28 @@ static int read_directory_contents(const char *path, struct string_list *list,
 
 	while ((e = readdir_skip_dot_and_dotdot(dir))) {
 		if (pathspec) {
+			int is_dir = 0;
+
 			strbuf_setlen(&match, len);
 			strbuf_addstr(&match, e->d_name);
+			if (dtype != DT_UNKNOWN) {
+				is_dir = dtype == DT_DIR;
+			} else {
+				struct stat st;
+				struct strbuf pathbuf = STRBUF_INIT;
+				strbuf_addstr(&pathbuf, path);
+				strbuf_complete(&pathbuf, '/');
+				strbuf_addstr(&pathbuf, e->d_name);
+				if (!lstat(&st, pathbuf.buf))
+					is_dir = S_ISDIR(st.st_mode);
+				else
+					; /* punt */
+				strbuf_release(&pathbuf);
+			}
 
 			if (!match_leading_pathspec(NULL, pathspec,
 						    match.buf, match.len,
-						    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))
+						    0, NULL, is_dir))
 				continue;
 		}
 
Collin Funk· Jun 18, 2025, 19:32 UTC · re: Junio C Hamano · lore

Re: [PATCH] portability: allow building in systems without d_type

Hi Junio,
Junio C Hamano <gitster@pobox.com> writes:
Show 9 quoted lines
> This may allow you to compile and build, but does the resulting
> binary do what you want it to?
> [...]
> On a platform without d_type member, DTYPE() macro gives DT_UNKNOWN
> that is not DT_DIR, so essentially you are always passing 0 even
> when you are looking at a directory (in which case you must pass 1)
> to match_leading_pathspec().
>
> So I somehow doubt this is a correct fix.

Good points. I guess I had reviewed the original patch too late to realize myself...

Show 41 quoted lines
> I do not know if get_dtype() helper function is easily applicable to
> this codepath, so I wrote this in a longhand...
>
>
>  diff-no-index.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git c/diff-no-index.c w/diff-no-index.c
> index 7c95222ba6..677df91fc5 100644
> --- c/diff-no-index.c
> +++ w/diff-no-index.c
> @@ -41,12 +41,28 @@ static int read_directory_contents(const char *path, struct string_list *list,
>  
>  	while ((e = readdir_skip_dot_and_dotdot(dir))) {
>  		if (pathspec) {
> +			int is_dir = 0;
> +
>  			strbuf_setlen(&match, len);
>  			strbuf_addstr(&match, e->d_name);
> +			if (dtype != DT_UNKNOWN) {
> +				is_dir = dtype == DT_DIR;
> +			} else {
> +				struct stat st;
> +				struct strbuf pathbuf = STRBUF_INIT;
> +				strbuf_addstr(&pathbuf, path);
> +				strbuf_complete(&pathbuf, '/');
> +				strbuf_addstr(&pathbuf, e->d_name);
> +				if (!lstat(&st, pathbuf.buf))
> +					is_dir = S_ISDIR(st.st_mode);
> +				else
> +					; /* punt */
> +				strbuf_release(&pathbuf);
> +			}
>  
>  			if (!match_leading_pathspec(NULL, pathspec,
>  						    match.buf, match.len,
> -						    0, NULL, DTYPE(e) == DT_DIR ? 1 : 0))
> +						    0, NULL, is_dir))
>  				continue;
>  		}
>  

Two very minor issues with with this patch. The arguments to 'lstat' are reversed and the 'dtype' variable is not declared. Here is a diff that I applied after your diff:

Show changes to diff-no-index.c +2 −1
diff --git a/diff-no-index.c b/diff-no-index.c
index 677df91fc5..a768b46dcd 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -42,6 +42,7 @@ static int read_directory_contents(const char *path, struct string_list *list,
 	while ((e = readdir_skip_dot_and_dotdot(dir))) {
 		if (pathspec) {
 			int is_dir = 0;
+			int dtype = DTYPE(e);
 
 			strbuf_setlen(&match, len);
 			strbuf_addstr(&match, e->d_name);
@@ -53,7 +54,7 @@ static int read_directory_contents(const char *path, struct string_list *list,
 				strbuf_addstr(&pathbuf, path);
 				strbuf_complete(&pathbuf, '/');
 				strbuf_addstr(&pathbuf, e->d_name);
-				if (!lstat(&st, pathbuf.buf))
+				if (!lstat(pathbuf.buf, &st))
 					is_dir = S_ISDIR(st.st_mode);
 				else
 					; /* punt */

With Carlo's patch the following tests fail:

    $ sh t4053-diff-no-index.sh
    [...]
    not ok 35 - diff --no-index with pathspec nested pathspec
    #       
    #               test_expect_code 1 git diff --name-status --no-index c d 1/2 >actual &&
    #               cat >expect <<-EOF &&
    #               D       c/1/2/a
    #               D       c/1/2/b
    #               EOF
    #               test_cmp expect actual
    #       
    not ok 36 - diff --no-index with pathspec glob
    #       
    #               test_expect_code 1 git diff --name-status --no-index c d ":(glob)**/a" >actual &&
    #               cat >expect <<-EOF &&
    #               D       c/1/2/a
    #               EOF
    #               test_cmp expect actual
    #       
    ok 37 - diff --no-index with pathspec glob and exclude
    # failed 2 among 37 test(s)

But with your patch (+ the minor corrections) the tests pass as
expected:

    $ sh t4053-diff-no-index.sh
    [...]
    ok 35 - diff --no-index with pathspec nested pathspec
    ok 36 - diff --no-index with pathspec glob
    ok 37 - diff --no-index with pathspec glob and exclude
    # passed all 37 test(s)

Therefore, I think your fix is good to go with a commit message and my
changes. Feel free to add me to Reviewed-by/Tested-By.

Collin

← back to recent threads