Re: sys_ioctl, sys_select: fix link errors
Hi Bruno, Bruno Haible writes: >> >* lib/sys_ioctl.in.h (ioctl): Fix declaration idiom. >> >> This patch appears to break Inetutils bootstrapping: >> >> http://hydra.nixos.org/build/335490 > > Thanks for the quick report. This should fix it. The fix is modeled after the > gettimeofday, forkpty, openpty wrappers by Eric. It does the trick, yes. Thanks! Ludo’.
Re: read() and write() for MinGW
Conceptually this is extremely simple, and I've found that gnulib already contains useful bits like SOCKET_TO_FD, FD_TO_SOCKET, and the code for determining if an arbitrary fd is a socket - which is all great. Yes, and the read() and write() functions don't need to make this distinction, because they call ReadFile and WriteFile, which work equally fine with HANDLEs and SOCKETs. ... but not for all sockets, only those created by gnulib's socket: /* We have to use WSASocket() to create non-overlapped IO sockets. Overlapped IO sockets cannot be used with read/write. */ fh = WSASocket (domain, type, protocol, NULL, 0, 0); Neil, can you make sure that all your sockets are gnulib-created? ((defined _WIN32 || defined __WIN32__)&& !defined __CYGWIN__) is the solution. (HAVE_WINSOCK2_H&& !defined __CYGWIN__) would be equivalent. Agreed. Paolo
[PATCH] save-cwd: don't leak a file descriptor when the caller execs.
Signed-off-by: James Youngman --- ChangeLog|7 +++ lib/save-cwd.c |2 ++ modules/save-cwd |1 + 3 files changed, 10 insertions(+), 0 deletions(-) diff --git a/ChangeLog b/ChangeLog index dca5a12..f9bd220 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2010-03-28 James Youngman + + save-cwd: don't leak a file descriptor when the caller execs. + * lib/save-cwd.c (save_cwd): set the close-on-exec flag for the + saved file descriptor. + * modules/save-cwd (Depends-on): Depend on cloexec. + 2010-03-28 Bruno Haible Ralf Wildenhues diff --git a/lib/save-cwd.c b/lib/save-cwd.c index 7394d50..cf43a35 100644 --- a/lib/save-cwd.c +++ b/lib/save-cwd.c @@ -31,6 +31,7 @@ #include "chdir-long.h" #include "unistd--.h" #include "xgetcwd.h" +#include "cloexec.h" #if GNULIB_FCNTL_SAFER # include "fcntl--.h" @@ -84,6 +85,7 @@ save_cwd (struct saved_cwd *cwd) return cwd->name ? 0 : -1; } + set_cloexec_flag (cwd->desc, true); return 0; } diff --git a/modules/save-cwd b/modules/save-cwd index 46a1276..aab5e5e 100644 --- a/modules/save-cwd +++ b/modules/save-cwd @@ -8,6 +8,7 @@ m4/save-cwd.m4 Depends-on: chdir-long +cloexec stdbool unistd-safer xgetcwd -- 1.5.6.5
Re: [PATCH] save-cwd: don't leak a file descriptor when the caller execs.
On 03/29/2010 11:49 AM, James Youngman wrote: Signed-off-by: James Youngman Good catch, thanks! Do you have commit access? Paolo
Re: [PATCH] save-cwd: don't leak a file descriptor when the caller execs.
On Mon, Mar 29, 2010 at 11:23 AM, Paolo Bonzini wrote: > On 03/29/2010 11:49 AM, James Youngman wrote: >> >> Signed-off-by: James Youngman > > Good catch, thanks! Do you have commit access? I don't. Thanks, James.
Re: [PATCH] save-cwd: don't leak a file descriptor when the caller execs.
James Youngman wrote: > + save-cwd: don't leak a file descriptor when the caller execs. > + * lib/save-cwd.c (save_cwd): set the close-on-exec flag for the > + saved file descriptor. > + * modules/save-cwd (Depends-on): Depend on cloexec. Thanks! I've pushed.
Re: [PATCH] build: avoid link failure on systems using gnulib's fcntl but not open
Bruno Haible wrote: > Replying to Jim I wrote: >> > "_rpl_open", referenced from: >> > _grepfile in grep.o >> The situation in the 'grep' package is as follows: >> - The main part, in lib/, has the module 'fcntl' and therefore >> implicitly 'fcntl-h', but not the 'open' module. >> - The tests part, in gnulib-tests/, has the module 'open', as a >> dependency from 'dup2-tests'. >> - The presence of the 'open' module is indicated through an >> AC_SUBSTed variable GNULIB_OPEN. There is only one GNULIB_OPEN >> for the entire package, and it is set to 1. >> - Therefore the lib/fcntl.h replacement defines "#define open rpl_open". >> >> I wouldn't say that it's a bug in gnulib. >> It's a limitation of gnulib that you cannot decide to not use a module in >> the main part but use it in the tests. If you really wanted that, you >> would need a setup with two different configure.ac files, one for the >> main part and one for the tests. > > But the user may be using open() in the main part, in lib/, without wanting > to use gnulib's 'open' module. We document for each module the list of > problems that it fixes. It's not because that dup2-tests requires an open() > function with a special workaround that the main part, in lib/, would need > it as well. > > So I think it's a bug in gnulib. ... Nice work. I'm glad you took the time to solve this properly.
Re: [PATCH] build: sync primary and tests-related gnulib module lists
Bruno Haible wrote: > Jim Meyering wrote: ... >> This patch is for information/discussion only. ... >> * bootstrap.conf (gnulib_modules): Synchronize the primary list >> of modules with the list used by tests, so that we don't have >> a repeat of the wctob-vs-Solaris 8 build failure. > > You should now be able to revert this. The bug is fixed in gnulib > since yesterday. Thanks, but as the hint from that message implied [reinserted above], I had no intention of pushing that change, so nothing needs to be reverted.
Re: [PATCH] exclude: fix the case of globs vs. EXCLUDE_INCLUDE
-BEGIN PGP SIGNED MESSAGE- Hash: RIPEMD160 On 03/29/2010 02:56 AM, Jim Meyering wrote: > Javier Villavicencio wrote: >> I think you may have mixed up the two patches I reported against grep > > Think again ;-) :+) I just did, you're right. > >> bug 29358, so, to clarify: >> That above is actually the patch that made most sense to me against the >> EXCLUDE_INCLUDE logic, as in: excluded_file_pattern_p should return >> !excluded on string match, not depending on the value excluded gets from >> options. >> >> The fix for GLOB matching is the second patch to grep's main.c, that >> just adds the EXCLUDE_WILDCARDS option to add_exclude(). > > Both are required. > If we don't use exclude_fnmatch properly, then --include=GLOB > doesn't work. If we don't specify EXCLUDE_WILDCARDS, then > grep doesn't even call exclude_fnmatch. > Yep, at some point debugging this I quite broke it in two separate things, _INCLUDE and _WILDCARDS, this one is indeed for both. Sorry for the spam. Salu2, Javier. -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.14 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEAREDAAYFAkuwjD8ACgkQgvV6MZSadQqN3ACfW38bX2XBQxAan1QDBYekeZJN Lx8AoIk2O0CyuC4Q/YDNjErTlj9nY6+R =bRqI -END PGP SIGNATURE-
make dist depending on binaries which use gnulib
Hi, it has just been pointed out to me that "make dist" for current head of GNU Zile fails, unless preceded by "make". This is because make dist distributes a file which can only be made once the zile binary has been built. I have two questions: 1. Is it expected that "make dist" should work without a preceding "make"? i.e. is this really a bug? 2. If so, then I have a further problem: if I simply make the file in question explicitly depend on zile$(EXEEXT), then zile is compiled by make dist, but cannot be linked because it needs ../lib/libgnu.a. How do I correctly make "make dist" build this? -- http://rrt.sc3d.org
Re: [PATCH 1/3] Macro _header_without_use renamed to _sc_header_without_use in maint.mk
Jose E. Marchesi wrote: > Subject: [PATCH 1/3] Macro _header_without_use renamed to > _sc_header_without_use in maint.mk Thanks, Jose. This looks pretty clear cut. I'll push this along with the others once I've tested.
Re: lchmod cygwin failure
On 03/25/2010 01:41 AM, Simon Josefsson wrote: > Building in cygwin fails because: > > depbase=`echo test-fcntl-h-c++.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\ > g++ -DHAVE_CONFIG_H -I. -I. -I. -I.. -I./.. -I../gllib -I./../gllib > -MT test-fcntl-h-c++.o -MD -MP -MF $depbase.Tpo -c -o test-fcntl-h-c++.o > test-fcntl-h-c++.cc &&\ > mv -f $depbase.Tpo $depbase.Po > In file included from ../gllib/fcntl.h:41, > from test-fcntl-h-c++.cc:22: > ../gllib/sys/stat.h:668: error: ‘lchmod’ was not declared in this scope > ../gllib/sys/stat.h:668: error: invalid type in declaration before ‘;’ > token > make[3]: *** [test-fcntl-h-c++.o] Error 1 > > (Not sure how the broken UTF-8 characters happened..) What terminal are you using? It is most likely a case of using a locale with a UTF-8 charset, but a terminal like rxvt that does not support multibyte characters (rxvt development has pretty much been abandoned upstream, but newer urxvt understands UTF-8). > > Any ideas? Looks like Bruno's patches fix the issue. Indeed, lchmod is an optional function - it exists in BSD, but is not standardized. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: macros factorization
On 03/27/2010 11:41 AM, Bruno Haible wrote: > +# gl_MODULE_INDICATOR_SET_VARIABLE([modulename]) > +# sets the shell variable that indicates the presence of the given module to > +# a C preprocessor expression that will evaluate to 1. > +AC_DEFUN([gl_MODULE_INDICATOR_SET_VARIABLE], > +[ > + > GNULIB_[]m4_translit([$1],[abcdefghijklmnopqrstuvwxyz./-],[ABCDEFGHIJKLMNOPQRSTUVWXYZ___])=1 Any objections to a followup patch that makes this fit in 80 columns? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 2/2] maint: use pragma consistently across replacement headers
On 03/28/2010 04:35 AM, Bruno Haible wrote: >>> Did you know that GCC has a special optimization for include files? ... > >> are you saying that the optimization is useless for those files, because they >> are split includes, and thus already fail to meet the pattern that gcc >> is looking for? > > No, on the contrary: The previous shape of most of our *.in.h files matches > what GCC is looking for: It starts with a "#ifndef _GL_STRING_H" and ends > with the corresponding "#endif". The fact that _GL_STRING_H is tested again > inside is irrelevant for this optimization. I think we're in violent agreement - for the few files with special invocation (like fcntl.h), we cannot satisfy gcc's optimization anyway, so we might as well guarantee the pragma is up front to avoid any issues with conditional optimization; but for the remaining files, we might as well satisfy gcc's optimization, since the effort is trivial. > Here's the proposed patch: Please apply. > +++ lib/arpa_inet.in.hSun Mar 28 12:07:53 2010 > @@ -16,12 +16,12 @@ > along with this program; if not, write to the Free Software Foundation, > Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ > > -# if __GNUC__ >= 3 > -...@pragma_system_header@ > -# endif > - > #ifndef _GL_ARPA_INET_H > > +#if __GNUC__ >= 3 > +...@pragma_system_header@ > +#endif Hmm, if we were using cppi more religiously, I wouldn't have introduced the whitespace usage in that #if in the first place. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH] save-cwd: don't leak a file descriptor when the caller execs.
On 03/29/2010 03:49 AM, James Youngman wrote:
> Signed-off-by: James Youngman
> ---
> ChangeLog|7 +++
> lib/save-cwd.c |2 ++
> modules/save-cwd |1 +
> 3 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index dca5a12..f9bd220 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,10 @@
> +2010-03-28 James Youngman
> +
> + save-cwd: don't leak a file descriptor when the caller execs.
> + * lib/save-cwd.c (save_cwd): set the close-on-exec flag for the
> + saved file descriptor.
> + * modules/save-cwd (Depends-on): Depend on cloexec.
Nice catch. Ultimately, though, it would be even nicer to guarantee
that we can do open (".", O_RDONLY | O_CLOEXEC), rather than having to
drag in "cloexec.h"; it is more efficient on POSIX 2008 systems.
[this email exists more as a reminder for when I get around to making
O_CLOEXEC work, than as a criticism of your patch]
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: make dist depending on binaries which use gnulib
On 03/29/2010 07:22 AM, Reuben Thomas wrote: > I have two questions: > > 1. Is it expected that "make dist" should work without a preceding > "make"? i.e. is this really a bug? The GNU Coding Standards requires 'make' before 'make dist', so it is not technically a bug. However, since 'make dist' is such a handy shortcut compared to 'make && make dist', many packages go out of their way to provide this additional guarantee above and beyond GCS. > > 2. If so, then I have a further problem: if I simply make the file in > question explicitly depend on zile$(EXEEXT), then zile is compiled by > make dist, but cannot be linked because it needs ../lib/libgnu.a. How > do I correctly make "make dist" build this? Is your SUBDIRS list of subdirectories in Makefile.am in dependency order? For example, be sure that you list lib prior to src, if src depends on lib being built. You can also add an explicit '.' if that helps. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 1/3] Macro _header_without_use renamed to _sc_header_without_use in maint.mk
> Subject: [PATCH 1/3] Macro _header_without_use renamed to _sc_header_without_use in maint.mk Thanks, Jose. This looks pretty clear cut. I'll push this along with the others once I've tested. Thanks :)
Re: make dist depending on binaries which use gnulib
Reuben Thomas wrote: > 1. Is it expected that "make dist" should work without a preceding > "make"? i.e. is this really a bug? Not technically, as Eric already explained. However, ... > 2. If so, then I have a further problem: if I simply make the file in > question explicitly depend on zile$(EXEEXT), then zile is compiled by > make dist, but cannot be linked because it needs ../lib/libgnu.a. How > do I correctly make "make dist" build this? I made some changes to accommodate that in cppi recently: http://git.savannah.gnu.org/cgit/cppi.git/commit/?id=49d4baaf363bc31b2a3 http://git.savannah.gnu.org/cgit/cppi.git/commit/?id=8a32cf9196e06b74e17 The latter was required only because I use help2man to generate man pages from --help.
Re: make dist depending on binaries which use gnulib
Hello Reuben, * Reuben Thomas wrote on Mon, Mar 29, 2010 at 03:22:08PM CEST: > it has just been pointed out to me that "make dist" for current head > of GNU Zile fails, unless preceded by "make". This is because make > dist distributes a file which can only be made once the zile binary > has been built. > > I have two questions: > > 1. Is it expected that "make dist" should work without a preceding > "make"? i.e. is this really a bug? The GNU Coding Standards usually only set requirements upon the behavior of your build system *when run from an extracted tarball*. This is crucial for being able to build from a readonly medium: imagine an extracted tarball on a CD or (more likely) on a readonly or shared mount. Then the use should be able to run configure in a temporary directory and build without an error, nor with race conditions when building in parallel in different build trees. Cheers, Ralf
Re: Detecting IP_TOS (and IPPROTO_IP vs. SOL_IP)
[moving to gnulib, replies can drop autoconf] On 03/28/2010 03:54 PM, Philip Prindeville wrote: >>> Calling the sequence: >>> >>> setsockopt(fd, , IP_TOS,&foo, sizeof(foo)); >>> >>> can be tricky. Linux uses SOL_IP for , whereas Solaris and BSD use >>> IPPROTO_IP for this instead. >>> >>> Not sure what MacOS or HP-UX do. >>> >>> Do the newer versions of autoconf have canned code for detecting what to >>> use? >>> >> No. >> >> My version of Linux defines both SOL_IP and IPPROTO_IP to the same >> value. Since changing it would seem to break the API, I *think* >> if looking at Linux alone you should be able to just go with IPPROTO_IP. >> Untested, you get to keep the pieces if it breaks. >> > > Except, of course, that most people widely agree that using the protocol > number as a synonym for the layer number was a poor design choice, and > one that was eventually corrected (via the introduction of SOL_IP). > > Thus I would argue the opposite: that always using SOL_IP is the correct > approach, i.e. > > #ifndef SOL_IP > #define SOL_IP IPPROTO_IP > #endif > ... > > r = setsockopt(s, SOL_IP,&qos, sizeof(qos)); > > > In summary, the API was already broken, and propagating that usage model > is probably not a good thing. > > > >>> And if not, can someone please add it? >>> >>> I just grepped the 2.65 tarball but didn't find anything in there. >>> >> Generally, for such issues it might be a good idea to just use gnulib if >> you can; but I just looked and it doesn't address this particular issue >> yet. Anyone want to tackle making gnulib give better SOL_IP support, for those platforms that don't yet implement it? -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/3] Macro _prohibit_regexp replaced by _sc_search_regep and rules adapted to use the new macro.
On 03/28/2010 08:54 AM, Jose E. Marchesi wrote:
>
>>From 24ab183f237468f2aa59d2424dc416f61c183671 Mon Sep 17 00:00:00 2001
> From: Jose E. Marchesi
> Date: Sun, 28 Mar 2010 15:42:03 +0200
> Subject: [PATCH 3/3] Macro _prohibit_regexp replaced by _sc_search_regep and
> rules adapted to use the new macro.
Typo (s/regep/regexp/). Also, that's an awfully long subject, which
later shows up in 'git log --oneline'. I would trim it to something
like the following (one line summary, then further details separated by
a blank line):
maint: replace _prohibit_regexp with _sc_search_regexp
All existing rules are adapted to use the new factorization, which
provides a more declarative syntax for pattern searching syntax checks.
> # Don't use Texinfo @acronym{} as it is not a good idea.
> sc_texinfo_acronym:
> - @if $(VC_LIST_EXCEPT) | grep -lE '\.texi$$' >/dev/null; then\
> - grep -nE '@acronym{'\
> - $$($(VC_LIST_EXCEPT) | grep -E '\.texi$$') && \
> - { echo '$(ME): found use of Texinfo @acronym{}' 1>&2; \
> - exit 1; } || :; \
> - else :; \
> - fi
> + @prohibit='@acronym{' \
> + in_vc_files='\.texi$$' \
> + halt='found use of Texinfo @acronym{}' \
> + $(_sc_search_regexp)
Jim mentioned that this rule should also look at *.txi and *.texinfo.
> # #if HAVE_... will evaluate to false for any non numeric string.
> # That would be flagged by using -Wundef, however gnulib currently
> # tests many undefined macros, and so we can't enable that option.
> # So at least preclude common boolean strings as macro values.
> sc_Wundef_boolean:
> - @test -e '$(CONFIG_INCLUDE)' && \
> -grep -Ei '^#define.*(yes|no|true|false)$$' '$(CONFIG_INCLUDE)' && \
> - { echo 'Use 0 or 1 for macro values' 1>&2; exit 1; } || :
> + @prohibit='^#define.*(yes|no|true|false)$$' \
> + in_vc_files='$(CONFIG_INCLUDE)' \
> + halt='Use 0 or 1 for macro values' \
> + $(_sc_search_regexp)
$(CONFIG_INCLUDE) is not a controlled file. You need to use in_files
instead.
Thanks again for doing this nice factorization.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Bug-tar] 1.23: FreeBSD 7 bug affects GNU tar
[adding bug-gnulib] On 03/29/2010 02:12 PM, Christian Weisgerber wrote: > There is a stupid, stupid bug in FreeBSD 7 (up to and including the > 7.3 release): The fdopendir() function is present in libc, but the > prototype is missing from . > > GNU tar's configure picks up fdopendir(), but since there is no > prototype, it ends up typed as > > int fdopendir(); > > This is bad. fdopendir() returns a pointer, which is truncated > from 64 to 32 bits on LP64 platforms. Running GNU tar's "make > check" leaves a trail of coredumps behind... > > The macro complex around m4/dirent*.m4 and m4/fdopendir.m4 needs > some sort of check if fdopendir() is declared, and if not, must > provide a prototype. These macros are probably used in other GNU > projects as well, and bug-tar may not be the right address, but I'm > throwing it out there in the hope that it will reach somebody who > knows how to deal with this properly. Yep, tar borrows this from gnulib. It shouldn't be too hard to address in gnulib, at which point, the next tar release to use a new gnulib will automatically be fixed. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [PATCH 3/3] Macro _prohibit_regexp replaced by _sc_search_regep and rules adapted to use the new macro.
Eric Blake wrote:
> On 03/28/2010 08:54 AM, Jose E. Marchesi wrote:
>>
>>>From 24ab183f237468f2aa59d2424dc416f61c183671 Mon Sep 17 00:00:00 2001
>> From: Jose E. Marchesi
>> Date: Sun, 28 Mar 2010 15:42:03 +0200
>> Subject: [PATCH 3/3] Macro _prohibit_regexp replaced by _sc_search_regep and
>> rules adapted to use the new macro.
>
> Typo (s/regep/regexp/). Also, that's an awfully long subject, which
> later shows up in 'git log --oneline'. I would trim it to something
> like the following (one line summary, then further details separated by
> a blank line):
>
> maint: replace _prohibit_regexp with _sc_search_regexp
>
> All existing rules are adapted to use the new factorization, which
> provides a more declarative syntax for pattern searching syntax checks.
>
>> # Don't use Texinfo @acronym{} as it is not a good idea.
>> sc_texinfo_acronym:
>> -@if $(VC_LIST_EXCEPT) | grep -lE '\.texi$$' >/dev/null; then\
>> -grep -nE '@acronym{'\
>> -$$($(VC_LIST_EXCEPT) | grep -E '\.texi$$') && \
>> - { echo '$(ME): found use of Texinfo @acronym{}' 1>&2; \
>> -exit 1; } || :; \
>> -else :; \
>> -fi
>> +@prohibit='@acronym{' \
>> +in_vc_files='\.texi$$' \
>> +halt='found use of Texinfo @acronym{}' \
>> + $(_sc_search_regexp)
>
> Jim mentioned that this rule should also look at *.txi and *.texinfo.
>
>> # #if HAVE_... will evaluate to false for any non numeric string.
>> # That would be flagged by using -Wundef, however gnulib currently
>> # tests many undefined macros, and so we can't enable that option.
>> # So at least preclude common boolean strings as macro values.
>> sc_Wundef_boolean:
>> -@test -e '$(CONFIG_INCLUDE)' && \
>> - grep -Ei '^#define.*(yes|no|true|false)$$' '$(CONFIG_INCLUDE)' && \
>> - { echo 'Use 0 or 1 for macro values' 1>&2; exit 1; } || :
>> +@prohibit='^#define.*(yes|no|true|false)$$' \
>> +in_vc_files='$(CONFIG_INCLUDE)' \
>> +halt='Use 0 or 1 for macro values' \
>> + $(_sc_search_regexp)
>
> $(CONFIG_INCLUDE) is not a controlled file. You need to use in_files
> instead.
>
> Thanks again for doing this nice factorization.
Good catch.
I've been testing this and will send feedback tomorrow.
Christian Weisgerber: [Bug-tar] 1.23: FreeBSD 7 bug affects GNU tar
Hello, Here's a report from one of GNU tar users. Regards, Sergey --- Forwarded message Date: Mon, 29 Mar 2010 22:12:02 +0200 From: Christian Weisgerber To: bug-...@gnu.org Message-ID: <20100329201202.ga28...@lorvorc.mips.inka.de> Subject: [Bug-tar] 1.23: FreeBSD 7 bug affects GNU tar There is a stupid, stupid bug in FreeBSD 7 (up to and including the 7.3 release): The fdopendir() function is present in libc, but the prototype is missing from . GNU tar's configure picks up fdopendir(), but since there is no prototype, it ends up typed as int fdopendir(); This is bad. fdopendir() returns a pointer, which is truncated from 64 to 32 bits on LP64 platforms. Running GNU tar's "make check" leaves a trail of coredumps behind... The macro complex around m4/dirent*.m4 and m4/fdopendir.m4 needs some sort of check if fdopendir() is declared, and if not, must provide a prototype. These macros are probably used in other GNU projects as well, and bug-tar may not be the right address, but I'm throwing it out there in the hope that it will reach somebody who knows how to deal with this properly. - -- Christian "naddy" Weisgerber na...@mips.inka.de --- End of Forwarded message
[PATCH] fdopendir: work around FreeBSD bug
Without a declaration, at least tar would core dump on 64-bit
FreeBSD because gcc only used 32 bits of the resulting pointer.
* m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): New witness.
* m4/fdopendir.m4 (gl_FUNC_FDOPENDIR): Set it.
* modules/dirent (Makefile.am): Substitute it.
* lib/dirent.in.h (fdopendir): Supply missing FreeBSD
declaration.
* doc/posix-functions/fdopendir.texi (fdopendir): Document the
fix.
Reported by Christian Weisgerber .
Signed-off-by: Eric Blake
---
I'm still trying to get access to a FreeBSD 7 box to test this out
myself (my current FreeBSD access is running 8.0, but that does not
have the bug). I'll probably push this tomorrow, hoping either for
some positive feedback or for a successful reproduction of the problem
myself in the meantime.
ChangeLog | 12
doc/posix-functions/fdopendir.texi |3 +++
lib/dirent.in.h|2 +-
m4/dirent_h.m4 |3 ++-
m4/fdopendir.m4|7 ++-
modules/dirent |1 +
6 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index b6f1dc1..1367dd0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2010-03-29 Eric Blake
+
+ fdopendir: work around FreeBSD bug
+ * m4/dirent_h.m4 (gl_DIRENT_H_DEFAULTS): New witness.
+ * m4/fdopendir.m4 (gl_FUNC_FDOPENDIR): Set it.
+ * modules/dirent (Makefile.am): Substitute it.
+ * lib/dirent.in.h (fdopendir): Supply missing FreeBSD
+ declaration.
+ * doc/posix-functions/fdopendir.texi (fdopendir): Document the
+ fix.
+ Reported by Christian Weisgerber .
+
2010-03-28 James Youngman
save-cwd: don't leak a file descriptor when the caller execs.
diff --git a/doc/posix-functions/fdopendir.texi
b/doc/posix-functions/fdopendir.texi
index fae7bb7..89b1a6a 100644
--- a/doc/posix-functions/fdopendir.texi
+++ b/doc/posix-functions/fdopendir.texi
@@ -17,6 +17,9 @@ fdopendir
that @samp{dirfd(fdopendir(n))==n} (dirfd might fail, or return a
different file descriptor than n).
@item
+This function exists but is not declared on some platforms:
+FreeBSD 7.3.
+...@item
This function does not reject non-directory file descriptors on some
platforms:
GNU/Hurd.
diff --git a/lib/dirent.in.h b/lib/dirent.in.h
index 15d488b..a7dd6d3 100644
--- a/lib/dirent.in.h
+++ b/lib/dirent.in.h
@@ -77,7 +77,7 @@ _GL_WARN_ON_USE (dirfd, "dirfd is unportable - "
_GL_FUNCDECL_RPL (fdopendir, DIR *, (int fd));
_GL_CXXALIAS_RPL (fdopendir, DIR *, (int fd));
# else
-# if !...@have_fdopendir@
+# if !...@have_fdopendir@ || !...@have_decl_fdopendir@
_GL_FUNCDECL_SYS (fdopendir, DIR *, (int fd));
# endif
_GL_CXXALIAS_SYS (fdopendir, DIR *, (int fd));
diff --git a/m4/dirent_h.m4 b/m4/dirent_h.m4
index 5f88a27..361296a 100644
--- a/m4/dirent_h.m4
+++ b/m4/dirent_h.m4
@@ -1,4 +1,4 @@
-# dirent_h.m4 serial 11
+# dirent_h.m4 serial 12
dnl Copyright (C) 2008-2010 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -46,6 +46,7 @@ AC_DEFUN([gl_DIRENT_H_DEFAULTS],
GNULIB_ALPHASORT=0; AC_SUBST([GNULIB_ALPHASORT])
dnl Assume proper GNU behavior unless another module says otherwise.
HAVE_DECL_DIRFD=1;AC_SUBST([HAVE_DECL_DIRFD])
+ HAVE_DECL_FDOPENDIR=1;AC_SUBST([HAVE_DECL_FDOPENDIR])
HAVE_FDOPENDIR=1; AC_SUBST([HAVE_FDOPENDIR])
HAVE_SCANDIR=1; AC_SUBST([HAVE_SCANDIR])
HAVE_ALPHASORT=1; AC_SUBST([HAVE_ALPHASORT])
diff --git a/m4/fdopendir.m4 b/m4/fdopendir.m4
index 7282d4b..1207917 100644
--- a/m4/fdopendir.m4
+++ b/m4/fdopendir.m4
@@ -1,4 +1,4 @@
-# serial 3
+# serial 4
# See if we need to provide fdopendir.
dnl Copyright (C) 2009-2010 Free Software Foundation, Inc.
@@ -35,4 +35,9 @@ AC_DEFUN([gl_FUNC_FDOPENDIR],
AC_LIBOBJ([fdopendir])
fi
fi
+ dnl FreeBSD 7.3 has the function, but failed to declare it.
+ AC_CHECK_DECLS([fdopendir], [], [HAVE_DECL_FDOPENDIR=0], [[
+#include
+#include
+]])
])
diff --git a/modules/dirent b/modules/dirent
index 9b99dbc..8783d83 100644
--- a/modules/dirent
+++ b/modules/dirent
@@ -31,6 +31,7 @@ dirent.h: dirent.in.h $(CXXDEFS_H) $(ARG_NONNULL_H)
$(WARN_ON_USE_H)
-e 's|@''GNULIB_SCANDIR''@|$(GNULIB_SCANDIR)|g' \
-e 's|@''GNULIB_ALPHASORT''@|$(GNULIB_ALPHASORT)|g' \
-e 's|@''HAVE_DECL_DIRFD''@|$(HAVE_DECL_DIRFD)|g' \
+ -e 's|@''HAVE_DECL_FDOPENDIR''@|$(HAVE_DECL_FDOPENDIR)|g' \
-e 's|@''HAVE_FDOPENDIR''@|$(HAVE_FDOPENDIR)|g' \
-e 's|@''HAVE_SCANDIR''@|$(HAVE_SCANDIR)|g' \
-e 's|@''HAVE_ALPHASORT''@|$(HAVE_ALPHASORT)|g' \
--
1.6.6.1
Re: read() and write() for MinGW
Paolo Bonzini writes: >>> Conceptually this is extremely simple, and I've found that gnulib >>> already contains useful bits like SOCKET_TO_FD, FD_TO_SOCKET, and the >>> code for determining if an arbitrary fd is a socket - which is all >>> great. >> >> Yes, and the read() and write() functions don't need to make this >> distinction, because they call ReadFile and WriteFile, which work >> equally fine with HANDLEs and SOCKETs. Thanks Bruno. I hope I understood your reply correctly. What I took from it was that read() and write() only don't work when they are called with the wrong type of ID - i.e. a SOCKET instead of a file descriptor - and that they are fine so long as the correct ID conversion is done, using _open_osfhandle(). I hope that's correct. > ... but not for all sockets, only those created by gnulib's socket: > > /* We have to use WSASocket() to create non-overlapped IO sockets. > Overlapped IO sockets cannot be used with read/write. */ > fh = WSASocket (domain, type, protocol, NULL, 0, 0); > > Neil, can you make sure that all your sockets are gnulib-created? I'm not sure. Guile's API, on both C and Scheme level, reflects the traditional Unix/Linux sockets API, which I believe has no equivalent of Windows' overlapped operation. So in that sense Guile has no requirement to support overlapped sockets. However... - MSDN says [1] that a non-blocking socket must be overlapped (if I'm understanding the article correctly), and it would be quite undesirable for Guile to exclude non-blocking socket operation on MinGW. [1] http://support.microsoft.com/kb/181611 - Guile's C API allows constructing a Scheme port around an arbitrary file descriptor - so ideally that would still work for any kind of Windows socket for which it's possible to obtain a file descriptor. Can you let me know your thoughts on those points? Thanks, Neil
Re: make dist depending on binaries which use gnulib
On 29 March 2010 19:56, Jim Meyering wrote: > Reuben Thomas wrote: >> 1. Is it expected that "make dist" should work without a preceding >> "make"? i.e. is this really a bug? > > Not technically, as Eric already explained. Yes, thanks Eric! (And yes, I had my SUBDIRS correctly ordered,) > The latter was required only because I use help2man > to generate man pages from --help. Me too, so I suspect your solution is exactly what I would need. Only, I'm happy to be GCS-compliant and require make before make dist for now. Thanks both of you. -- http://rrt.sc3d.org
Re: [PATCH] fdopendir: work around FreeBSD bug
[re-adding the list]
On 03/29/2010 04:39 PM, Christian Weisgerber wrote:
>> I'm still trying to get access to a FreeBSD 7 box to test this out
>> myself (my current FreeBSD access is running 8.0, but that does not
>> have the bug).
>
> Well, you can temporarily remove the prototype for fdopendir() from
> .
>
> Your changes look correct to me, and I've wedged them into GNU tar
> 1.23 on my FreeBSD 7.3/amd64 box and they work.
Thanks for the feedback. Yeah, I suppose I could temporarily corrupt a
non-FreeBSD system for an equally effective test.
>
> Minor remarks:
>
>> --- a/m4/fdopendir.m4
>> +++ b/m4/fdopendir.m4
>> @@ -35,4 +35,9 @@ AC_DEFUN([gl_FUNC_FDOPENDIR],
>>AC_LIBOBJ([fdopendir])
>> fi
>>fi
>> + dnl FreeBSD 7.3 has the function, but failed to declare it.
>> + AC_CHECK_DECLS([fdopendir], [], [HAVE_DECL_FDOPENDIR=0], [[
>> +#include
>> +#include
>> +]])
>> ])
>
> The is useless; the preceding test only uses it to pull
> in O_RDONLY. *BSD documents that should be included
> before , but in practice that isn't necessary.
Thanks for the pointers. We haven't yet encountered a system where
in isolation fails to compile, but I will fix the
inclusion.
>
> If you are really paranoid, you need to provide a prototype here...
>
> [int fd = open ("conftest.c", O_RDONLY);
> if (fd < 0) return 2;
> return !!fdopendir (fd);])],
>
> ... because fdopendir() could return a valid pointer whose lower
> 32 bits just happen to be 0, as unlikely as that is in practice.
> But then only some versions of FreeBSD lack the prototype, and
> FreeBSD's fdopendir() does not accept a file handle from a non-directory
> file.
Worth incorporating into my patch, then. I'll post my respin, once I've
tested locally.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
duplocale cygwin failure (was: Re: lchmod cygwin failure)
Bruno Haible writes: > Fixed as follows: > > > 2010-03-27 Bruno Haible > > Fix a compilation error on Cygwin with g++ >= 4.3. > * lib/sys_stat.in.h (lchmod): Don't warn about the use of this function > if it is undefined or if we alias it to chmod. > (lstat): Don't warn about the use of this function if it is undefined > or if we alias it to stat. > Reported by Simon Josefsson. Thanks! The same class of problem appears to occur for duplocale too: In file included from test-locale-c++.cc:22: ../gllib/locale.h:329: error: ‘duplocale’ was not declared in this scope ../gllib/locale.h:329: error: invalid type in declaration before ‘;’ token make[3]: *** [test-locale-c++.o] Error 1 make[3]: Leaving directory `/home/Simon/gnulib/build/gltests' See http://autobuild.josefsson.org/gnulib/log-201003291132136972000.txt /Simon
Re: lchmod cygwin failure
Eric Blake writes: >> ../gllib/sys/stat.h:668: error: ‘lchmod’ was not declared in this scope ... >> (Not sure how the broken UTF-8 characters happened..) > > What terminal are you using? It is most likely a case of using a locale > with a UTF-8 charset, but a terminal like rxvt that does not support > multibyte characters (rxvt development has pretty much been abandoned > upstream, but newer urxvt understands UTF-8). FWIW, I cut'n'pasted the text from a browser window. I've now made the web server send a charset=UTF-8 by default. The auto-guessing code in my browser (GNU IceCat) apparently didn't work well without a charset parameter, so it incorrectly guessed the file was Latin-1, leading to the broken data above. /Simon
Re: [PATCH 2/2] maint: use pragma consistently across replacement headers
Hi Eric, > but for the remaining files, we might as > well satisfy gcc's optimization, since the effort is trivial. > > Please apply. Applied. > Hmm, if we were using cppi more religiously, I wouldn't have introduced > the whitespace usage in that #if in the first place. The file lib/time.in.h is intended according to cppi, and I find it quite unesthetic. Bruno
