[PATCH] portability: allow building in systems without d_type

[PATCH] portability: allow building in systems without d_type

[Carlo Marcelo Arenas Belón]

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;
 		}
 
-- 
2.50.0.53.g63c9ac04f7

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

[Collin Funk]

Hi Carlo,

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

> 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

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

[Marc Branchaud]

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.


> 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;
>   		}
>   

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

[Kristoffer Haugsbakk]

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

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

[Junio C Hamano]

Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:

> 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(-)

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;
 		}
 

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

[Collin Funk]

Hi Junio,

Junio C Hamano <gitster@pobox.com> writes:

> 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...

> 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:

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