bug#10733: 24.0.93; w32 file truncation

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Cc: lekktu@gmail.com,  Takaaki.Ota@am.sony.com,  10733@debbugs.gnu.org
> Date: Mon, 06 Feb 2012 18:58:15 +0100
> 
> >> /* 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.

No, I'm not.

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

No.  The `stat' Emacs uses is in w32.c.  What you see on
lib-src/ntlib.c is just a minimal subset of what we have in w32.c.

> Symlinks are detected and handled specially on MSVCRT's stat. In
> aessence, for symlinks it uses fstat.

But fstat probably calls GetFileInformationByHandle under the hood,
and we already call that function in w32.c:stat.  So maybe the fix is
not as ugly and inelegant as you thought.

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

This doesn't work.  I don't remember all the details at the moment,
but it's not a coincidence we define `stat' in w32.c, not `sys_stat',
as we do with other functions.  One immediate problem with this is
that we also have our own implementation of `fstat' on w32.c, and they
must both share the same `struct stat'.  It gets really ugly, believe
me.

Anyway, I don't think we need to call MSVCRT's `fstat' to fix this,
see above.

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

See above: you are looking at the wrong `stat'.  The one that caused
all this is on w32.c.

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

It wasn't a "removal", it was a tricky bug.

Anyway, see this discussion:

  http://lists.gnu.org/archive/html/emacs-devel/2011-07/msg00785.html
  http://lists.gnu.org/archive/html/emacs-devel/2011-07/msg00827.html