Subject: Re: bug#10733: 24.0.93; w32 file truncation

From: Óscar Fuentes <[email-masked]>
Date: Mon, 06 Feb 2012 18:58:15 +0100

Eli Zaretskii <[email-masked]> writes:

>> > I bet it is a bug on the CRT (the stat call that retrieves the file
>> > size, to be precise). Maybe it is a MinGW thing.
>> 
>> No, it is an Emacs thing. `stat' is defined in lib-src/ntlib.c,
>> overriding the MSVCRT implementation, which accounts for symlinks, while
>> Emacs' does not.
>
> Can you tell the details, please?  Specifically, what would it take to
> "account for symlinks" in our implementation of `stat'?  You did say
> symlinks were supposed to be transparent.

Transparent for applications, yes. The CRT is not an application. If you
replace CRT functions with your own, the transparency goes away.

>> Before the definition of `stat' on lib-src/ntlib.c there is this
>> comment:
>> 
>> /* We need this because nt/inc/sys/stat.h defines struct stat that is
>>    incompatible with the MS run-time libraries.  */
>> 
>> That looks like an understatement. Actually, we need our own stat
>> function and struct because the `struct stat' that Emacs uses is
>> incompatible with the one defined in MSVCRT, right?
>
> No, you are missing the point of that comment.  lib-src/ntlib.c is not
> compiled into Emacs, it's compiled into lib-src programs.
> Theoretically, since those programs don't need anything fancy from
> `stat', they could use the stock MSVCRT implementation.  But because
> these programs are compiled with -I../nt/inc, the compiler picks up
> the definition of `struct stat' that is used by Emacs, and because of
> this incompatibility lib-src programs cannot use the MSVCRT
> implementation of `stat'.

I think you are saying essentially the same as I do but with very
different words.

OTOH, you are saying "lib-src/ntlib.c is not compiled into Emacs, it's
compiled into lib-src programs." and the key hypotheses made by me here
is that Emacs uses the `stat' definition on lib-src/ntlib.c. Is that
correct?

>> The obvious fix does not seem difficult, although ugly and
>> verbose.
>
> Can you please describe the problem, in addition to what you propose
> to be a solution?

Symlinks are detected and handled specially on MSVCRT's stat. In
aessence, for symlinks it uses fstat. It all amounts to 15 lines of
simple code. But that's not directly transposable to Emacs' stat because
we need to access and translate the MSVCRT `struct stat' that `fstat'
creates to the Emacs' `struct stat'. That final part is the "ugly and
verbose" part.

>> I'll like to remove the Emacs reimplementation of `stat'
>
> That is a non-starter.  The private implementation of `stat' is needed
> to support Posix features, such as meaningful inode numbers, UID and
> GID, etc.  You won't find anything close to that in MSVCRT.  Quite a
> few parts in Emacs expect those features.
>
>> How much time we have until the release?
>
> We cannot afford to make such a change before the release, no matter
> how far away is it, even if I'd agree to that (which I don't).  `stat'
> is too central to Emacs operation to make such changes at this time.

Such change would be conceptually straightforward and quite safe. It
amounts to using MSVCRT `stat' and translating its `struct stat' to
Emacs' `struct stat'. Instead of defining our own `stat' and `struct
stat' we define `emacs_w32_stat' and `struct emacs_w32_stat' and do

#if windows
#define stat emacs_w32_stat
#endif

so no changes are required outside lib-src/ntlib.c and nt/sys/stat.h.

I don't get your mention to inode numbers, UID and GID. The
implementation on ntlib.c simply does

  buf->st_ino = 0;

and I see no references to UID and GID. Maybe those are obtained
elsewhere. As far as I can see a tranlation from MSVCRT's `struct stat'
to Emacs' is possible.

>> BTW, the obvious fix may require some care for not breaking Emacs
>> support on MS Windows versions prior to XP. We still support Windows 9x,
>> don't we?
>
> Yes, we do.  In fact, Emacs 24.1 will again work on Windows 9X, after
> it turned out that 23.x (and perhaps also 22.x) didn't.

So we are reintroducing support for a legacy OS after being de facto
removed for several years. Curious.