* bug#10733: 24.0.93; w32 file truncation @ 2012-02-05 22:34 Ota, Takaaki 2012-02-05 23:51 ` Juanma Barranquero 0 siblings, 1 reply; 27+ messages in thread From: Ota, Takaaki @ 2012-02-05 22:34 UTC (permalink / raw) To: 10733 When opening a file on an NTFS file system the file opens as if the content is truncated to size of 65375 characters. This happens when the file is an NTFS symbolic link which is made by mklink command of cmd.exe. There is no problem if the target file of the NTFS symbolic link is smaller than this size. -Tak --text follows this line-- This bug report will be sent to the Bug-GNU-Emacs mailing list and the GNU bug tracker at debbugs.gnu.org. Please check that the From: line contains a valid email address. After a delay of up to one day, you should receive an acknowledgement at that address. Please write in English if possible, as the Emacs maintainers usually do not have translators for other languages. Please describe exactly what actions triggered the bug, and the precise symptoms of the bug. If you can, give a recipe starting from `emacs -Q': If Emacs crashed, and you have the Emacs process in the gdb debugger, please include the output from the following gdb commands: `bt full' and `xbacktrace'. For information about debugging Emacs, please read the file c:/emacs/emacs-24.0.93/etc/DEBUG. In GNU Emacs 24.0.93.1 (i386-mingw-nt6.1.7601) of 2012-01-29 on TAK-VAIO-Z1290 Windowing system distributor `Microsoft Corp.', version 6.1.7601 Configured using: `configure --with-gcc (4.5) --cflags -Ic:/d/pub/emacs/include --ldflags -Lc:/d/pub/emacs/lib' Important settings: value of $LC_ALL: nil value of $LC_COLLATE: nil value of $LC_CTYPE: nil value of $LC_MESSAGES: nil value of $LC_MONETARY: nil value of $LC_NUMERIC: nil value of $LC_TIME: nil value of $LANG: ENU value of $XMODIFIERS: nil locale-coding-system: cp1252 default enable-multibyte-characters: t Major mode: Fundamental Minor modes in effect: tooltip-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Recent input: ESC x f i n d SPC f i SPC <return> m e m o <return> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <next> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> <down> ESC : ( p o i n t ) <return> ESC : r e p o r t SPC C-g ESC x r e p o r SPC e m SPC SPC <return> Recent messages: For information about GNU Emacs and the GNU system, type C-h C-a. You can run the command `find-file' with C-x C-f scroll-up-command: End of buffer [21 times] byte-code: End of buffer [21 times] 65375 (#o177537, #xff5f) Quit Load-path shadows: None found. Features: (shadow sort gnus-util mail-extr message format-spec rfc822 mml easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mailabbrev mail-utils gmm-utils mailheader emacsbug time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel dos-w32 disp-table ls-lisp w32-win w32-vars tool-bar dnd fontset image fringe lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer button faces cus-face files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote make-network-process multi-tty emacs) ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-05 22:34 bug#10733: 24.0.93; w32 file truncation Ota, Takaaki @ 2012-02-05 23:51 ` Juanma Barranquero 2012-02-06 0:16 ` Ota, Takaaki 2012-02-06 0:20 ` Óscar Fuentes 0 siblings, 2 replies; 27+ messages in thread From: Juanma Barranquero @ 2012-02-05 23:51 UTC (permalink / raw) To: Ota, Takaaki; +Cc: 10733 > When opening a file on an NTFS file system the file opens as if the > content is truncated to size of 65375 characters. This happens when > the file is an NTFS symbolic link which is made by mklink command of > cmd.exe. There is no problem if the target file of the NTFS symbolic > link is smaller than this size. Can you please give a step-by-step recipe, starting from "emacs -Q"? Thanks, Juanma ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-05 23:51 ` Juanma Barranquero @ 2012-02-06 0:16 ` Ota, Takaaki 2012-02-06 4:05 ` Eli Zaretskii 2012-02-06 0:20 ` Óscar Fuentes 1 sibling, 1 reply; 27+ messages in thread From: Ota, Takaaki @ 2012-02-06 0:16 UTC (permalink / raw) To: lekktu; +Cc: 10733 Yes, I can do that. 1. Have a text file that has more than 65375 characters. Let's call this text file test.txt. 2. Run cmd.exe and change directory to where test.txt is located. 3. Then execute the next command in the cmd. mklink link.txt test.txt 4. Now run emacs as "emacs -Q link.txt" and do M-> (end-of-buffer). 5. Find that you are not seeing the end of the file but somewhere around 65375 character location and the rest of the file is not visible. -Tak Sun, 5 Feb 2012 15:51:40 -0800: Juanma Barranquero <lekktu@gmail.com> wrote: > > When opening a file on an NTFS file system the file opens as if the > > content is truncated to size of 65375 characters. This happens when > > the file is an NTFS symbolic link which is made by mklink command of > > cmd.exe. There is no problem if the target file of the NTFS symbolic > > link is smaller than this size. > > Can you please give a step-by-step recipe, starting from "emacs -Q"? > > Thanks, > > Juanma > ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 0:16 ` Ota, Takaaki @ 2012-02-06 4:05 ` Eli Zaretskii 2012-02-06 5:25 ` Óscar Fuentes 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 4:05 UTC (permalink / raw) To: Ota, Takaaki; +Cc: lekktu, 10733 > Date: Sun, 5 Feb 2012 16:16:23 -0800 > From: "Ota, Takaaki" <Takaaki.Ota@am.sony.com> > Cc: 10733@debbugs.gnu.org > > Yes, I can do that. > > 1. Have a text file that has more than 65375 characters. Let's call > this text file test.txt. > > 2. Run cmd.exe and change directory to where test.txt is located. > > 3. Then execute the next command in the cmd. > > mklink link.txt test.txt > > 4. Now run emacs as "emacs -Q link.txt" and do M-> (end-of-buffer). > > 5. Find that you are not seeing the end of the file but somewhere > around 65375 character location and the rest of the file is not > visible. Emacs on Windows doesn't support symlinks; they are new with Vista and Windows 7. Someone needs to write the code to support that, before this can be considered a bug (as opposed to a feature request). What you describe seems to indicate that insert-file-contents only reads the first 64KB portion of the file, and then bails out. Running Emacs under debugger and reporting where it bails out of the reading loop might help coming up with some band-aid solution. But full support of symlink requires specific support code, that just isn't there at this time. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 4:05 ` Eli Zaretskii @ 2012-02-06 5:25 ` Óscar Fuentes 2012-02-06 16:16 ` Óscar Fuentes 2012-02-06 16:55 ` Eli Zaretskii 0 siblings, 2 replies; 27+ messages in thread From: Óscar Fuentes @ 2012-02-06 5:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, Ota, Takaaki, 10733 Eli Zaretskii <eliz@gnu.org> writes: [snip] > Emacs on Windows doesn't support symlinks; they are new with Vista and > Windows 7. Someone needs to write the code to support that, before > this can be considered a bug (as opposed to a feature request). > > What you describe seems to indicate that insert-file-contents only > reads the first 64KB portion of the file, and then bails out. Running > Emacs under debugger and reporting where it bails out of the reading > loop might help coming up with some band-aid solution. But full > support of symlink requires specific support code, that just isn't > there at this time. NTFS symlinks are transparent to applications. Emacs doesn't need to know that it is accessing a symlink to correctly read and save the file. 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. It would be interesting to hear reports from people using an Emacs compiled with a modern version of Visual Studio, or even know if people using the most recent MinGW CRT release can duplicate the problem. This is a very serious bug (potential data loss). I'll put this on my TODO list and hopefully run a debug session on a Windows 7 machine next week. Meanwhile, if anyone can provide the info mentioned on the previous paragraph, it would be helpful. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 5:25 ` Óscar Fuentes @ 2012-02-06 16:16 ` Óscar Fuentes 2012-02-06 17:21 ` Eli Zaretskii 2012-02-06 16:55 ` Eli Zaretskii 1 sibling, 1 reply; 27+ messages in thread From: Óscar Fuentes @ 2012-02-06 16:16 UTC (permalink / raw) To: 10733; +Cc: lekktu, Ota, Takaaki Óscar Fuentes <ofv@wanadoo.es> 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. 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? The obvious fix does not seem difficult, although ugly and verbose. OTOH, I'll like to remove the Emacs reimplementation of `stat', but that looks more cumbersome. How much time we have until the release? 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? ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 16:16 ` Óscar Fuentes @ 2012-02-06 17:21 ` Eli Zaretskii 2012-02-06 17:58 ` Óscar Fuentes 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 17:21 UTC (permalink / raw) To: Óscar Fuentes; +Cc: lekktu, Takaaki.Ota, 10733 > From: Óscar Fuentes <ofv@wanadoo.es> > Cc: Eli Zaretskii <eliz@gnu.org>, lekktu@gmail.com, "Ota\, Takaaki" <Takaaki.Ota@am.sony.com> > Date: Mon, 06 Feb 2012 17:16:52 +0100 > > Óscar Fuentes <ofv@wanadoo.es> 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. > 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'. > 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? > 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. > 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 17:21 ` Eli Zaretskii @ 2012-02-06 17:58 ` Óscar Fuentes 2012-02-06 18:37 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Óscar Fuentes @ 2012-02-06 17:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, Takaaki.Ota, 10733 Eli Zaretskii <eliz@gnu.org> 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. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 17:58 ` Óscar Fuentes @ 2012-02-06 18:37 ` Eli Zaretskii 2012-02-06 20:19 ` Ota, Takaaki 2012-02-06 23:27 ` Óscar Fuentes 0 siblings, 2 replies; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 18:37 UTC (permalink / raw) To: Óscar Fuentes; +Cc: lekktu, Takaaki.Ota, 10733 > 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 ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 18:37 ` Eli Zaretskii @ 2012-02-06 20:19 ` Ota, Takaaki 2012-02-06 20:23 ` Lennart Borgman ` (2 more replies) 2012-02-06 23:27 ` Óscar Fuentes 1 sibling, 3 replies; 27+ messages in thread From: Ota, Takaaki @ 2012-02-06 20:19 UTC (permalink / raw) To: eliz; +Cc: ofv, lekktu, 10733 For your information: (file-attributes "memo") (nil 1 544 513 (20269 7834) (20269 7834) (20269 7834) 0 "-rw-rw-rw-" nil (8448 8 . 22758) (62004 . 15649)) (file-attributes "memo.old") (nil 1 544 513 (19686 63524) (20262 20973) (19686 63524) 233153 "-r--r--r--" nil (512 5 . 24351) (62004 . 15649)) where "memo" is the NTFS symlink and "memo.old" is a real file. I don't know how the size 0 on symlink side is translated into 64K. -Tak Mon, 6 Feb 2012 10:37:56 -0800: Eli Zaretskii <eliz@gnu.org> wrote: > > 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 > ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 20:19 ` Ota, Takaaki @ 2012-02-06 20:23 ` Lennart Borgman 2012-02-06 21:11 ` Eli Zaretskii 2012-02-06 20:24 ` Lars Ingebrigtsen 2012-02-06 21:08 ` Eli Zaretskii 2 siblings, 1 reply; 27+ messages in thread From: Lennart Borgman @ 2012-02-06 20:23 UTC (permalink / raw) To: Ota, Takaaki; +Cc: ofv, lekktu, 10733 To me it seems quite scary that Emacs has its own implementation of such low level functions. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 20:23 ` Lennart Borgman @ 2012-02-06 21:11 ` Eli Zaretskii 2012-02-06 21:20 ` Lennart Borgman 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 21:11 UTC (permalink / raw) To: Lennart Borgman; +Cc: ofv, lekktu, Takaaki.Ota, 10733 > From: Lennart Borgman <lennart.borgman@gmail.com> > Date: Mon, 6 Feb 2012 21:23:20 +0100 > Cc: eliz@gnu.org, ofv@wanadoo.es, lekktu@gmail.com, 10733@debbugs.gnu.org > > To me it seems quite scary that Emacs has its own implementation of > such low level functions. Don't worry: `stat' is a low-level function on Posix platforms, but not on Windows. w32.c is full of such "low-level" functions, reimplemented to be more Posix like, because MSVCRT does a very poor job in that regard, and Emacs expects Posix-compliance in quite a few places. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 21:11 ` Eli Zaretskii @ 2012-02-06 21:20 ` Lennart Borgman 2012-02-06 21:31 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Lennart Borgman @ 2012-02-06 21:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ofv, lekktu, Takaaki.Ota, 10733 On Mon, Feb 6, 2012 at 22:11, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Lennart Borgman <lennart.borgman@gmail.com> >> Date: Mon, 6 Feb 2012 21:23:20 +0100 >> Cc: eliz@gnu.org, ofv@wanadoo.es, lekktu@gmail.com, 10733@debbugs.gnu.org >> >> To me it seems quite scary that Emacs has its own implementation of >> such low level functions. > > Don't worry: `stat' is a low-level function on Posix platforms, but > not on Windows. That is not what worries me. This is reimplementation is surely necessary. What worries me is the bypassing of MSVCRT. I think that perhaps it is better to reimplement on top of that. But that might be just me. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 21:20 ` Lennart Borgman @ 2012-02-06 21:31 ` Eli Zaretskii 2012-02-06 21:35 ` Lennart Borgman 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 21:31 UTC (permalink / raw) To: Lennart Borgman; +Cc: ofv, lekktu, Takaaki.Ota, 10733 > From: Lennart Borgman <lennart.borgman@gmail.com> > Date: Mon, 6 Feb 2012 22:20:31 +0100 > Cc: Takaaki.Ota@am.sony.com, ofv@wanadoo.es, lekktu@gmail.com, > 10733@debbugs.gnu.org > > What worries me is the bypassing of MSVCRT. Why? What do you think `stat' in MSVCRT does? (You can find the sources on the Internet.) It calls some of the same Win32 APIs, and then fills `struct stat'. There''s nothing magic about that, and nothing to be afraid of. > I think that perhaps it is better to reimplement on top of that. You can't. To get the missing information you must go to lower-level APIs. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 21:31 ` Eli Zaretskii @ 2012-02-06 21:35 ` Lennart Borgman 0 siblings, 0 replies; 27+ messages in thread From: Lennart Borgman @ 2012-02-06 21:35 UTC (permalink / raw) To: Eli Zaretskii; +Cc: ofv, lekktu, Takaaki.Ota, 10733 On Mon, Feb 6, 2012 at 22:31, Eli Zaretskii <eliz@gnu.org> wrote: >> From: Lennart Borgman <lennart.borgman@gmail.com> >> Date: Mon, 6 Feb 2012 22:20:31 +0100 >> Cc: Takaaki.Ota@am.sony.com, ofv@wanadoo.es, lekktu@gmail.com, >> 10733@debbugs.gnu.org >> >> What worries me is the bypassing of MSVCRT. > > Why? What do you think `stat' in MSVCRT does? (You can find the > sources on the Internet.) It calls some of the same Win32 APIs, and > then fills `struct stat'. There''s nothing magic about that, and > nothing to be afraid of. Ok, I see. But what seems to have happened now is that changes in these calls that are implemented in MSVCRT are not mirrored in the Emacs reimplementation. (And we are aware that it is a bit serious.) >> I think that perhaps it is better to reimplement on top of that. > > You can't. To get the missing information you must go to lower-level > APIs. Do you say that it would slow down the Emacs implementation to get this information with new calls (beside those potentially going through MSVCRT)? Then I agree. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 20:19 ` Ota, Takaaki 2012-02-06 20:23 ` Lennart Borgman @ 2012-02-06 20:24 ` Lars Ingebrigtsen 2012-02-06 21:09 ` Eli Zaretskii 2012-02-06 21:08 ` Eli Zaretskii 2 siblings, 1 reply; 27+ messages in thread From: Lars Ingebrigtsen @ 2012-02-06 20:24 UTC (permalink / raw) To: Ota, Takaaki; +Cc: ofv, lekktu, 10733 "Ota, Takaaki" <Takaaki.Ota@am.sony.com> writes: > where "memo" is the NTFS symlink and "memo.old" is a real file. I > don't know how the size 0 on symlink side is translated into 64K. If Emacs doesn't know the size of the file (i.e., the fs says that the file is 0 bytes long), then Emacs will only read the first 64K of the file. This was discussed in the bug report about how /proc files are truncated by Emacs. The fix proposed there (i.e., "read until you get to eof") would probably fix this, too. -- (domestic pets only, the antidote for overdose, milk.) http://lars.ingebrigtsen.no * Sent from my Rome ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 20:24 ` Lars Ingebrigtsen @ 2012-02-06 21:09 ` Eli Zaretskii 0 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 21:09 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: ofv, lekktu, Takaaki.Ota, 10733 > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: <eliz@gnu.org>, ofv@wanadoo.es, lekktu@gmail.com, 10733@debbugs.gnu.org > Date: Mon, 06 Feb 2012 21:24:04 +0100 > > "Ota, Takaaki" <Takaaki.Ota@am.sony.com> writes: > > > where "memo" is the NTFS symlink and "memo.old" is a real file. I > > don't know how the size 0 on symlink side is translated into 64K. > > If Emacs doesn't know the size of the file (i.e., the fs says that the > file is 0 bytes long), then Emacs will only read the first 64K of the > file. This was discussed in the bug report about how /proc files are > truncated by Emacs. > > The fix proposed there (i.e., "read until you get to eof") would > probably fix this, too. That's a band-aid at best. If Emacs doesn't know the size of the file that is the target of the symlink and its other attributes, other places will break. What is needed is to resolve the link, i.e. do the equivalent of `lstat'. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 20:19 ` Ota, Takaaki 2012-02-06 20:23 ` Lennart Borgman 2012-02-06 20:24 ` Lars Ingebrigtsen @ 2012-02-06 21:08 ` Eli Zaretskii 2 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 21:08 UTC (permalink / raw) To: Ota, Takaaki; +Cc: ofv, lekktu, 10733 > Date: Mon, 6 Feb 2012 12:19:53 -0800 > CC: <ofv@wanadoo.es>, <lekktu@gmail.com>, <10733@debbugs.gnu.org> > From: "Ota, Takaaki" <Takaaki.Ota@am.sony.com> > > For your information: > > (file-attributes "memo") > (nil 1 544 513 (20269 7834) (20269 7834) (20269 7834) 0 "-rw-rw-rw-" nil (8448 8 . 22758) (62004 . 15649)) > (file-attributes "memo.old") > (nil 1 544 513 (19686 63524) (20262 20973) (19686 63524) 233153 "-r--r--r--" nil (512 5 . 24351) (62004 . 15649)) > > where "memo" is the NTFS symlink and "memo.old" is a real file. I > don't know how the size 0 on symlink side is translated into 64K. insert-file-contents reads files in 64KB chunks. So it reads only one chunk. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 18:37 ` Eli Zaretskii 2012-02-06 20:19 ` Ota, Takaaki @ 2012-02-06 23:27 ` Óscar Fuentes 2012-02-06 23:43 ` Ota, Takaaki 2012-02-07 4:02 ` Eli Zaretskii 1 sibling, 2 replies; 27+ messages in thread From: Óscar Fuentes @ 2012-02-06 23:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, Takaaki.Ota, 10733 Eli Zaretskii <eliz@gnu.org> writes: [snip] > 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. That's important. Thanks. >> 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. Yup. This patch fixes the problem: diff --git a/src/w32.c b/src/w32.c index f610a36..035e1f2 100644 --- a/src/w32.c +++ b/src/w32.c @@ -3444,6 +3444,29 @@ stat (const char * path, struct stat * buf) } } + buf->st_size = 0; + + if ((wfd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && + (wfd.dwReserved0 == IO_REPARSE_TAG_SYMLINK)) + { + HANDLE fh; + BY_HANDLE_FILE_INFORMATION info; + + if ((fh = CreateFile (name, 0, 0, NULL, OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, NULL)) + && GetFileInformationByHandle (fh, &info)) + { + buf->st_size = info.nFileSizeHigh; + buf->st_size <<= 32; + buf->st_size += info.nFileSizeLow; + } + else + { + errno = ENOENT; + return -1; + } + } + if (!(NILP (Vw32_get_true_file_attributes) || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) /* No access rights required to get info. */ @@ -3532,9 +3555,12 @@ stat (const char * path, struct stat * buf) buf->st_dev = volume_info.serialnum; buf->st_rdev = volume_info.serialnum; - buf->st_size = wfd.nFileSizeHigh; - buf->st_size <<= 32; - buf->st_size += wfd.nFileSizeLow; + if (! buf->st_size) + { + buf->st_size = wfd.nFileSizeHigh; + buf->st_size <<= 32; + buf->st_size += wfd.nFileSizeLow; + } /* Convert timestamps to Unix format. */ buf->st_mtime = convert_time (wfd.ftLastWriteTime); Maybe it can be integrated in the if (!(NILP(Vw32_get_true_file_attributes) ... hence reusing the calls to CreateFile and GetFileInformationByHandle and shortening the patch, but as I don't know what Vw32_get_true_file_attributes does, preferread to follow the safe way. However, we still have another implementation on ntlib.c. And the fix is just for the size. Don't know if there are other attributes suffer from the same problem. Possibly the right thing is to do what MSVCRT does: if is-symlink? use fstat fi ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 23:27 ` Óscar Fuentes @ 2012-02-06 23:43 ` Ota, Takaaki 2012-02-07 0:07 ` Óscar Fuentes 2012-02-07 4:02 ` Eli Zaretskii 1 sibling, 1 reply; 27+ messages in thread From: Ota, Takaaki @ 2012-02-06 23:43 UTC (permalink / raw) To: ofv; +Cc: lekktu, 10733 Don't you need to dispose fh properly? -Tak Mon, 6 Feb 2012 15:27:07 -0800: Óscar Fuentes <ofv@wanadoo.es> wrote: > Eli Zaretskii <eliz@gnu.org> writes: > > [snip] > > > 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. > > That's important. Thanks. > > >> 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. > > Yup. This patch fixes the problem: > > diff --git a/src/w32.c b/src/w32.c > index f610a36..035e1f2 100644 > --- a/src/w32.c > +++ b/src/w32.c > @@ -3444,6 +3444,29 @@ stat (const char * path, struct stat * buf) > } > } > > + buf->st_size = 0; > + > + if ((wfd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && > + (wfd.dwReserved0 == IO_REPARSE_TAG_SYMLINK)) > + { > + HANDLE fh; > + BY_HANDLE_FILE_INFORMATION info; > + > + if ((fh = CreateFile (name, 0, 0, NULL, OPEN_EXISTING, > + FILE_FLAG_BACKUP_SEMANTICS, NULL)) > + && GetFileInformationByHandle (fh, &info)) > + { > + buf->st_size = info.nFileSizeHigh; > + buf->st_size <<= 32; > + buf->st_size += info.nFileSizeLow; > + } > + else > + { > + errno = ENOENT; > + return -1; > + } > + } > + > if (!(NILP (Vw32_get_true_file_attributes) > || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) > /* No access rights required to get info. */ > @@ -3532,9 +3555,12 @@ stat (const char * path, struct stat * buf) > buf->st_dev = volume_info.serialnum; > buf->st_rdev = volume_info.serialnum; > > - buf->st_size = wfd.nFileSizeHigh; > - buf->st_size <<= 32; > - buf->st_size += wfd.nFileSizeLow; > + if (! buf->st_size) > + { > + buf->st_size = wfd.nFileSizeHigh; > + buf->st_size <<= 32; > + buf->st_size += wfd.nFileSizeLow; > + } > > /* Convert timestamps to Unix format. */ > buf->st_mtime = convert_time (wfd.ftLastWriteTime); > > > Maybe it can be integrated in the > > if (!(NILP(Vw32_get_true_file_attributes) ... > > hence reusing the calls to CreateFile and GetFileInformationByHandle and > shortening the patch, but as I don't know what > Vw32_get_true_file_attributes does, preferread to follow the safe way. > > However, we still have another implementation on ntlib.c. > > And the fix is just for the size. Don't know if there are other > attributes suffer from the same problem. Possibly the right thing is to > do what MSVCRT does: > > if is-symlink? > use fstat > fi > ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 23:43 ` Ota, Takaaki @ 2012-02-07 0:07 ` Óscar Fuentes 0 siblings, 0 replies; 27+ messages in thread From: Óscar Fuentes @ 2012-02-07 0:07 UTC (permalink / raw) To: Ota, Takaaki; +Cc: ofv, lekktu, 10733 "Ota, Takaaki" <Takaaki.Ota@am.sony.com> writes: > Don't you need to dispose fh properly? Yes. And check that it works for directories too. The patch is not intended as a definitive fix, but as a data point on the discussion. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 23:27 ` Óscar Fuentes 2012-02-06 23:43 ` Ota, Takaaki @ 2012-02-07 4:02 ` Eli Zaretskii 2012-02-07 5:22 ` Óscar Fuentes 1 sibling, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2012-02-07 4:02 UTC (permalink / raw) To: Óscar Fuentes; +Cc: lekktu, Takaaki.Ota, 10733 > From: Óscar Fuentes <ofv@wanadoo.es> > Cc: lekktu@gmail.com, Takaaki.Ota@am.sony.com, 10733@debbugs.gnu.org > Date: Tue, 07 Feb 2012 00:27:07 +0100 > > >> 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. > > Yup. This patch fixes the problem: Thanks. > Maybe it can be integrated in the > > if (!(NILP(Vw32_get_true_file_attributes) ... > > hence reusing the calls to CreateFile and GetFileInformationByHandle and > shortening the patch, but as I don't know what > Vw32_get_true_file_attributes does, preferread to follow the safe way. You did right: w32-get-true-file-attributes can be set by the user to nil, if she wants her file ops faster. > And the fix is just for the size. Don't know if there are other > attributes suffer from the same problem. Possibly the right thing is to > do what MSVCRT does: > > if is-symlink? > use fstat > fi Since fstat is also reimplemented, I'd rather do what it does inline. For that, we need to know which other attributes are reported different. Or maybe just test for the reparse point up front and do all the work for the target instead. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-07 4:02 ` Eli Zaretskii @ 2012-02-07 5:22 ` Óscar Fuentes 2012-02-07 17:35 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Óscar Fuentes @ 2012-02-07 5:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, Takaaki.Ota, 10733 Eli Zaretskii <eliz@gnu.org> writes: >> Maybe it can be integrated in the >> >> if (!(NILP(Vw32_get_true_file_attributes) ... >> >> hence reusing the calls to CreateFile and GetFileInformationByHandle and >> shortening the patch, but as I don't know what >> Vw32_get_true_file_attributes does, preferread to follow the safe way. > > You did right: w32-get-true-file-attributes can be set by the user to > nil, if she wants her file ops faster. I was thinking on something like diff --git a/src/w32.c b/src/w32.c index 3d3d334..418be63 100644 --- a/src/w32.c +++ b/src/w32.c @@ -3447,8 +3447,12 @@ stat (const char * path, struct stat * buf) } } - if (!(NILP (Vw32_get_true_file_attributes) - || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) + buf->st_size = 0; + + if ((!(NILP (Vw32_get_true_file_attributes) + || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) + || ((wfd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && + (wfd.dwReserved0 == IO_REPARSE_TAG_SYMLINK))) /* No access rights required to get info. */ && (fh = CreateFile (name, 0, 0, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL)) @@ -3461,6 +3465,9 @@ stat (const char * path, struct stat * buf) if (GetFileInformationByHandle (fh, &info)) { + buf->st_size = info.nFileSizeHigh; + buf->st_size <<= 32; + buf->st_size += info.nFileSizeLow; buf->st_nlink = info.nNumberOfLinks; /* Might as well use file index to fake inode values, but this is not guaranteed to be unique unless we keep a handle open @@ -3535,9 +3542,12 @@ stat (const char * path, struct stat * buf) buf->st_dev = volume_info.serialnum; buf->st_rdev = volume_info.serialnum; - buf->st_size = wfd.nFileSizeHigh; - buf->st_size <<= 32; - buf->st_size += wfd.nFileSizeLow; + if (!buf->st_size) + { + buf->st_size = wfd.nFileSizeHigh; + buf->st_size <<= 32; + buf->st_size += wfd.nFileSizeLow; + } /* Convert timestamps to Unix format. */ buf->st_mtime = convert_time (wfd.ftLastWriteTime); That is not as robust, though, because if GetFileInformationByHandle fails the function does not return with an error code. >> And the fix is just for the size. Don't know if there are other >> attributes suffer from the same problem. Possibly the right thing is to >> do what MSVCRT does: >> >> if is-symlink? >> use fstat >> fi > > Since fstat is also reimplemented, I'd rather do what it does inline. > > For that, we need to know which other attributes are reported > different. Or maybe just test for the reparse point up front and do > all the work for the target instead. Since Emacs' fstat reimplementation is based on GetFileInformationByHandle, and that the handle points to the linked file (CreateFile follows the link unless told otherwise), we should be safe delegating all work to `fstat' when a symlink is detected on `stat' (the executable bit must be setted on `stat', but that's no problem.) ^ permalink raw reply related [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-07 5:22 ` Óscar Fuentes @ 2012-02-07 17:35 ` Eli Zaretskii 2012-08-03 10:59 ` Eli Zaretskii 0 siblings, 1 reply; 27+ messages in thread From: Eli Zaretskii @ 2012-02-07 17:35 UTC (permalink / raw) To: Óscar Fuentes; +Cc: lekktu, Takaaki.Ota, 10733 > From: Óscar Fuentes <ofv@wanadoo.es> > Cc: lekktu@gmail.com, Takaaki.Ota@am.sony.com, 10733@debbugs.gnu.org > Date: Tue, 07 Feb 2012 06:22:12 +0100 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> Maybe it can be integrated in the > >> > >> if (!(NILP(Vw32_get_true_file_attributes) ... > >> > >> hence reusing the calls to CreateFile and GetFileInformationByHandle and > >> shortening the patch, but as I don't know what > >> Vw32_get_true_file_attributes does, preferread to follow the safe way. > > > > You did right: w32-get-true-file-attributes can be set by the user to > > nil, if she wants her file ops faster. > > I was thinking on something like > > diff --git a/src/w32.c b/src/w32.c > index 3d3d334..418be63 100644 > --- a/src/w32.c > +++ b/src/w32.c > @@ -3447,8 +3447,12 @@ stat (const char * path, struct stat * buf) > } > } > > - if (!(NILP (Vw32_get_true_file_attributes) > - || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) > + buf->st_size = 0; > + > + if ((!(NILP (Vw32_get_true_file_attributes) > + || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) > + || ((wfd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && > + (wfd.dwReserved0 == IO_REPARSE_TAG_SYMLINK))) Then what made you hesitate? This approach looks fine to me. > >> if is-symlink? > >> use fstat > >> fi > > > > Since fstat is also reimplemented, I'd rather do what it does inline. > > > > For that, we need to know which other attributes are reported > > different. Or maybe just test for the reparse point up front and do > > all the work for the target instead. > > Since Emacs' fstat reimplementation is based on > GetFileInformationByHandle, and that the handle points to the linked > file (CreateFile follows the link unless told otherwise), we should be > safe delegating all work to `fstat' when a symlink is detected on `stat' > (the executable bit must be setted on `stat', but that's no problem.) Please compare w32.c's `fstat' with `stat', and you will see that the former does much less than the latter, even with information that can be gotten by the handle. To go the way you suggest, we need first to make `fstat' a proper subset of `stat'. (I never had time to do it, and since `fstat' is used much less that `stat' in Emacs, more important jobs came first.) ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-07 17:35 ` Eli Zaretskii @ 2012-08-03 10:59 ` Eli Zaretskii 0 siblings, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2012-08-03 10:59 UTC (permalink / raw) To: ofv, lekktu, Takaaki.Ota; +Cc: 10733-done > Date: Tue, 07 Feb 2012 19:35:25 +0200 > From: Eli Zaretskii <eliz@gnu.org> > Cc: lekktu@gmail.com, Takaaki.Ota@am.sony.com, 10733@debbugs.gnu.org > > > From: Óscar Fuentes <ofv@wanadoo.es> > > Cc: lekktu@gmail.com, Takaaki.Ota@am.sony.com, 10733@debbugs.gnu.org > > Date: Tue, 07 Feb 2012 06:22:12 +0100 > > > > Eli Zaretskii <eliz@gnu.org> writes: > > > > >> Maybe it can be integrated in the > > >> > > >> if (!(NILP(Vw32_get_true_file_attributes) ... > > >> > > >> hence reusing the calls to CreateFile and GetFileInformationByHandle and > > >> shortening the patch, but as I don't know what > > >> Vw32_get_true_file_attributes does, preferread to follow the safe way. > > > > > > You did right: w32-get-true-file-attributes can be set by the user to > > > nil, if she wants her file ops faster. > > > > I was thinking on something like > > > > diff --git a/src/w32.c b/src/w32.c > > index 3d3d334..418be63 100644 > > --- a/src/w32.c > > +++ b/src/w32.c > > @@ -3447,8 +3447,12 @@ stat (const char * path, struct stat * buf) > > } > > } > > > > - if (!(NILP (Vw32_get_true_file_attributes) > > - || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) > > + buf->st_size = 0; > > + > > + if ((!(NILP (Vw32_get_true_file_attributes) > > + || (EQ (Vw32_get_true_file_attributes, Qlocal) && is_slow_fs (name))) > > + || ((wfd.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) && > > + (wfd.dwReserved0 == IO_REPARSE_TAG_SYMLINK))) > > Then what made you hesitate? This approach looks fine to me. > > > >> if is-symlink? > > >> use fstat > > >> fi > > > > > > Since fstat is also reimplemented, I'd rather do what it does inline. > > > > > > For that, we need to know which other attributes are reported > > > different. Or maybe just test for the reparse point up front and do > > > all the work for the target instead. > > > > Since Emacs' fstat reimplementation is based on > > GetFileInformationByHandle, and that the handle points to the linked > > file (CreateFile follows the link unless told otherwise), we should be > > safe delegating all work to `fstat' when a symlink is detected on `stat' > > (the executable bit must be setted on `stat', but that's no problem.) > > Please compare w32.c's `fstat' with `stat', and you will see that the > former does much less than the latter, even with information that can > be gotten by the handle. To go the way you suggest, we need first to > make `fstat' a proper subset of `stat'. (I never had time to do it, > and since `fstat' is used much less that `stat' in Emacs, more > important jobs came first.) Since trunk revision 109416 adds full support for symlinks on MS-Windows platforms, and fixes this bug as a side effect, I'm closing this bug. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-06 5:25 ` Óscar Fuentes 2012-02-06 16:16 ` Óscar Fuentes @ 2012-02-06 16:55 ` Eli Zaretskii 1 sibling, 0 replies; 27+ messages in thread From: Eli Zaretskii @ 2012-02-06 16:55 UTC (permalink / raw) To: Óscar Fuentes; +Cc: lekktu, Takaaki.Ota, 10733 > From: Óscar Fuentes <ofv@wanadoo.es> > Cc: "Ota\, Takaaki" <Takaaki.Ota@am.sony.com>, lekktu@gmail.com, 10733@debbugs.gnu.org > Date: Mon, 06 Feb 2012 06:25:32 +0100 > > NTFS symlinks are transparent to applications. Well, evidently, they aren't. > Emacs doesn't need to know that it is accessing a symlink to > correctly read and save the file. Evidently, it does. ^ permalink raw reply [flat|nested] 27+ messages in thread
* bug#10733: 24.0.93; w32 file truncation 2012-02-05 23:51 ` Juanma Barranquero 2012-02-06 0:16 ` Ota, Takaaki @ 2012-02-06 0:20 ` Óscar Fuentes 1 sibling, 0 replies; 27+ messages in thread From: Óscar Fuentes @ 2012-02-06 0:20 UTC (permalink / raw) To: Juanma Barranquero; +Cc: Ota, Takaaki, 10733 Juanma Barranquero <lekktu@gmail.com> writes: >> When opening a file on an NTFS file system the file opens as if the >> content is truncated to size of 65375 characters. This happens when >> the file is an NTFS symbolic link which is made by mklink command of >> cmd.exe. There is no problem if the target file of the NTFS symbolic >> link is smaller than this size. > > Can you please give a step-by-step recipe, starting from "emacs -Q"? I can reproduce the problem on Windows 7 64 bits: * Run a cmd shell as Administrator (mklink is a privileged command). * Navigate to a directory with a text file larger than 64 KB (let's suppose that the file is named foo.txt). * mklink bar.txt foo.txt. * runemacs -Q bar.txt * The file appears truncated in Emacs. Very odd. ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2012-08-03 10:59 UTC | newest] Thread overview: 27+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-02-05 22:34 bug#10733: 24.0.93; w32 file truncation Ota, Takaaki 2012-02-05 23:51 ` Juanma Barranquero 2012-02-06 0:16 ` Ota, Takaaki 2012-02-06 4:05 ` Eli Zaretskii 2012-02-06 5:25 ` Óscar Fuentes 2012-02-06 16:16 ` Óscar Fuentes 2012-02-06 17:21 ` Eli Zaretskii 2012-02-06 17:58 ` Óscar Fuentes 2012-02-06 18:37 ` Eli Zaretskii 2012-02-06 20:19 ` Ota, Takaaki 2012-02-06 20:23 ` Lennart Borgman 2012-02-06 21:11 ` Eli Zaretskii 2012-02-06 21:20 ` Lennart Borgman 2012-02-06 21:31 ` Eli Zaretskii 2012-02-06 21:35 ` Lennart Borgman 2012-02-06 20:24 ` Lars Ingebrigtsen 2012-02-06 21:09 ` Eli Zaretskii 2012-02-06 21:08 ` Eli Zaretskii 2012-02-06 23:27 ` Óscar Fuentes 2012-02-06 23:43 ` Ota, Takaaki 2012-02-07 0:07 ` Óscar Fuentes 2012-02-07 4:02 ` Eli Zaretskii 2012-02-07 5:22 ` Óscar Fuentes 2012-02-07 17:35 ` Eli Zaretskii 2012-08-03 10:59 ` Eli Zaretskii 2012-02-06 16:55 ` Eli Zaretskii 2012-02-06 0:20 ` Óscar Fuentes
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).