git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] MinGW readdir reimplementation to support d_type

From
Johannes Sixt <j6t@kdbg.org>
Date
Apr 11, 2009, 21:44 UTC
Message-ID
<49E10F5A.9010400@kdbg.org>
In-Reply-To
<49DEFA30.1000101@gmail.com>
Marius Storm-Olsen schrieb:
Show 39 quoted lines
> Johannes Sixt said the following on 09.04.2009 22:34:
>> Marius Storm-Olsen schrieb:
>>> +struct mingw_dirent
>>> +{
>>> +    long        d_ino;            /* Always zero. */
>>> +    union {
>>> +        unsigned short    d_reclen;    /* Always zero. */
>>> +        unsigned char   d_type;        /* Reimplementation adds this */
>>> +    };
>>
>> VERY sneaky! I was wondering why you could get away without replacing
>> opendir and closedir, and why you still defined a replacement
>> mingw_DIR that contains the replacement mingw_dirent, until I noticed
>> this unnamed union.
>>
>> Since we don't use d_reclen anywhere in the code, wouldn't you get
>> away with
>>
>> #define d_type d_reclen
>>
>> unless the type (short vs. char) makes a difference. Or would you say
>> that doing that would be even more sneaky?
> 
> I'm sure it could be done just with a define. However, given the 
> remaining unused variables, I was wondering about also packing in 
> permission bits and file modification time in there, to optimize the 
> status checking even further. That way, on Windows, we would only need 
> one 'readdir' pass to check the whole repository, with no lstats 
> whatsoever. So, this was patch was a 'primer' for that, hence the union 
> with a proper uchar for the d_type.
> 
> However, that would also mean a significant change in the status 
> checking code, as it first lstat's ever file in the index, then uses 
> read_directory + lstat's for others. I guess that'll be too big of a 
> change in core code, so the vision is moot?
> 
> I'd be ok to just use the define, provided that it compiles cleanly of 
> course, if the above seems too ambitious. :-) I kinda feel like the 
> current code is more clean though :)
With a comment in the commit message, it would have been clear, perhaps.

I'll carry this in my (private) tree for a while with the below squashed in to avoid a lot of warnings.

-- Hannes
diff --git a/compat/mingw.h b/compat/mingw.h
index 104b310..16ec76b 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -260,4 +260,5 @@ struct mingw_dirent
  };
  #define dirent mingw_dirent
  #define readdir(x) mingw_readdir(x)
+struct dirent *mingw_readdir(DIR *dir);
  #endif // !NO_MINGW_REPLACE_READDIR
Previous: Marius Storm-OlsenNext: Heiko Voigt
Message 4 of 6 in “MinGW readdir reimplementation to support d_type”
  1. MinGW readdir reimplementation to support d_typeMarius Storm-Olsen, Apr 8, 2009
  2. Johannes SixtApr 9, 2009
  3. Marius Storm-OlsenApr 10, 2009
  4. Johannes SixtApr 11, 2009
  5. Heiko VoigtMay 7, 2009
  6. Marius Storm-OlsenMay 8, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.