Menu
▾
▴
[Tcl-bugs] [ tcl-Bugs-3598300 ] unix: tcl.h does not include sys/stat.h
|
From: SourceForge.net <no...@so...> - 2012-12-27 20:25:53
|
Bugs item #3598300, was opened at 2012-12-24 00:14 Message generated for change (Comment added) made by nijtmans You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3598300&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 52. Portability Support Group: current: 8.5.13 Status: Open Resolution: None Priority: 5 Private: No Submitted By: rofl0r (roflz) Assigned to: Jan Nijtmans (nijtmans) Summary: unix: tcl.h does not include sys/stat.h Initial Comment: tcl.h typedefs Tcl_StatBuf to struct stat/stat64 but does not include <sys/stat.h> this leads to compilation error in tclCmdAH.c because the storage size of variables of type Tcl_StatBuf is unknown. ---------------------------------------------------------------------- >Comment By: Jan Nijtmans (nijtmans) Date: 2012-12-27 12:25 Message: Thanks, Don! But I'm fully aware of this header reform, and I don't think that my latest suggestions violatest this: <http://core.tcl.tk/tcl/info/dcbf22ad99> I don't see any danger in this, but please put in your view! ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2012-12-27 11:56 Message: Only skimming the comments here. One thing to keep in mind with any editing of the #include dependency graph is to preserve the goals of the 8.5 era "header reform." See 922727. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-25 09:53 Message: Sorry, but digging a platform-specific header out and sticking it on top of the main generic one doesn't sound like a solid basis... Instead, I'd rather understand in detail why some parts of tcl*Port (like the compat/ ones) depend on being after tcl.h, and either stop this dependency, or move them in a separate file, to be included after tcl.h Can you please give these details (or just the kind of macros from tcl.h that are involed) ? ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2012-12-25 04:16 Message: > Jan, any comment on the side-effects you feared, so that you didn't > generalize ? Well, that change was for fixing a compiler warming with mingw, not for cygwin, but I agree with your analysis. It would be good to include sys/stat.h before tcl.h. The problem with this is the compat/*.h files, which currently need tcl.h to be included first. On cygwin/linux/solaris those are not used, so the move would be safe. But on older systems ..... Another remark: The inclusion order didn't change between Tcl 8.5 and 8.6 (I can see in Makefile.tcl that the error occurred in 8.6, but I am assuming the same 'bug' is in 8.5 as well). The best solution here might be to move the sys/stat.h include from tcl*Port.h to tclInt.h. See: <http://core.tcl.tk/tcl/info/02a51cbb69> That has the least probability to break on any other platform. rofl0r, Alex, would that work for you? Any problems you can see with this change? ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 17:43 Message: Transferring to Jan Nijtmans, who did a similar move in a specific case (cygwin). Jan, any comment on the side-effects you feared, so that you didn't generalize ? (I'm referring to ad7cfb6ee3 where you moved tclWinPort above tcl.h, which looks like a good idea of wider applicability ;) ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 17:30 Message: here's the bug: tclPort.h #include "tcl.h" #if !defined(_WIN32) # include "tclUnixPort.h" #endif it should include tclUnixPort.h before tcl.h, problem solved and for maximum portability put the include block in tclUnixport before any typedefs and macro definitions ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 17:25 Message: again i have not tweaked anything. this is the Makefile that gets created when running CFLAGS=-D_GNU_SOURCE ./configure. i never used anything else. it is perfectly legal to define _GNU_SOURCE, the libc is free to do it automatically for example by defining it in features, and so am i. since stat64 is completely non-standard, the implementation is free to define it as macro so you better make sure it works, by putting the inclusion of sys/stat.h before your typedefs. ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 17:19 Message: i mean this part here #elif defined(HAVE_STRUCT_STAT64) # define TclOSstat stat64 # define TclOSlstat lstat64 #else # define TclOSstat stat # define TclOSlstat lstat #endif the inclusion of stat.h comes later so whatever this tries to do, it doesn't see the stat64 macro and thus may fail. it is probably not related to the error i'm seeing. however it is very likely that the problem we're facing here has the same cause: tcl.h gets included before <sys/stat.h>, so the macro doesnt get expanded and the compiler fails to find the struct stat64. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 16:43 Message: >looking at tclUnixPort.h, it appears to me that sys/stat.h should be >included before any stat-related ifdeffery >there is code that defines things to be stat64, but since sys/stat.h is >included later, the stat64 macro does not get expanded. Red herring. This code is just a pair of prototypes, passing opaque pointers. Even if I agree that they don't benefit from the macro, there is no harm done beyond a possible warning. If you want to proceed on this, please give the error message that you go when trying to compile with the vanilla Makefile generated by configure (not the one tweaked with whatever habits you have). ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 16:40 Message: > it is not legal to include bits/stat.h directly, > this is an internal header! -if you do this, > that would explain the breakage Ahem. Do you realize that this is the deed of (your/musl's) sys/stat.h ? Tcl just includes <sys/stat.h> just as everybody else. ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 11:41 Message: looking at tclUnixPort.h, it appears to me that sys/stat.h should be included before any stat-related ifdeffery there is code that defines things to be stat64, but since sys/stat.h is included later, the stat64 macro does not get expanded. ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 11:28 Message: musl's sys/stat.h includes an arch specific bits/stat.h that provides the actual definition of struct stat. http://git.musl-libc.org/cgit/musl/tree/arch/i386/bits/stat.h however it is not legal to include bits/stat.h directly, this is an internal header! if you do this, that would explain the breakage. ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 11:25 Message: i never used any other Makefile adding -D_GNU_SOURCE to CFLAGS is something i automatically do always so it was not added specifically to circumvent some problems here. this is the exact Makefile that causes the build failure. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 10:26 Message: Still, there might be a side effect to -D_GNU_SOURCE. Please give the vanilla Makefile, the one lacking HAVE_SYS_STAT. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 10:24 Message: Ok, just seen the attachment, sorry ;) ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 10:23 Message: So what ? Now your sys/stat.h is included, and stat64 gets mapped to stat, Tcl ends up needing a "struct stat" somewhere. I assume it is in <bits/stat.h> as in most unices, isn't it. To make this exchange more efficient, please provide the generated Makefile without any tweaks. Patching tcl.h or forcing CFLAGS sound brittle ; better fix the root problem in the configure logic. ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 09:36 Message: here's the relevant header part in musl libc: http://git.musl-libc.org/cgit/musl/tree/include/sys/stat.h#n98 i added -D_GNU_SOURCE to the CFLAGS when launching configure, so this part got visible. tcl.h takes the typedef struct stat64 Tcl_StatBuf route. however without seeing the macro in sys/stat.h this cannot succeed, because stat64 is only a macro. ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 08:45 Message: Please attach the Makefile generated by Tcl's configure in this environment. ---------------------------------------------------------------------- Comment By: rofl0r (roflz) Date: 2012-12-24 05:02 Message: this was experienced using musl libc 0.9.8 on i386 since musl always uses LARGEFILE64, it #defines stat64 stat https://github.com/rofl0r/sabotage/commit/70807c9a16ef053b5bb432c69bccb29da27b1b10 ---------------------------------------------------------------------- Comment By: Alexandre Ferrieux (ferrieux) Date: 2012-12-24 01:44 Message: The unix-dependent includes come through tclUnixPort.h, included (through tclPort.h) by tclInt.h. So it is not as plain as a failure to include an obviously needed system header, but rather some ifdeffery that fails in this specific case (that I cannot reproduce). Any specific info re the OS and/or configure/-D flags ? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=3598300&group_id=10894 |