Re: gcc's procopen.c
[Removed many CCs, changed subject] Bruce Korb wrote: > Please have a look at GCC's fixincludes. > It needs (and uses) a chain of processes. > See gcc/fixincludes/server.[ch] and ../procopen.c > > To solve problems I couldn't figure out, I had to fork() and exit() > the writer to the chain, or the intermediate processes would hang. > Even with closing all the pipes and sending down HUP ioctls. > Nothing I tried worked until the original pipe controlling process > actually exited. This is an indication that some of the file descriptors leaked to processes that should not have them open. Remember, when you have a pipe from A to B, these file descriptors should be open only in A and B, not in any other process. Because if, for example, the writing fd of A is also open in another process C, and B does a read(), the OS does not know whether it should wake up A or C. It will wake up one of them, and if that happens to be C, the situation hangs, because C is not programmed to write anything to this fd. > In any event, it ought not be much trouble to > fiddle the interface so you could give this pipe-filter process > an initial FD pair so you could chain a bunch of them together. If it's not so much trouble, and if you need it, can you propose an API that would be 1. robust and 2. portable between Unix and Woe32 ? Bruno
wait-process API limitation
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Right now, wait_subprocess assumes that an exit status of 127 from the child process implies failure, and prints a message unless null_stderr is set. However, this led to a regression in m4 1.4.13 - an intentional status of 127 is ambiguous with failure only if the child had no output, and although m4 knows there was output, wait-process does not: $ echo 'esyscmd(echo `dnl'\''; exit 127)sysval' | m4-1.4.11 127 $ echo 'esyscmd(echo `dnl'\''; exit 127)sysval' | m4-1.4.13 m4-1.4.13: esyscmd subprocess failed 127 The fix from m4's point of view is to pass null_stderr=false to create_pipe_in, but null_stderr=true to wait_subprocess, but this feels like a bit of a hack, because of the inconsistency in the named parameter. Maybe it would be worth an API change to wait_subprocess to add an additional bool parameter, status_127_ok, which silences this particular error message if the calling process knows for certain that the child process produced output (or even if the parent process does not know this, but will be handling status 127 itself without additional output from the gnulib module). Or, maybe it is worth just renaming the existing parameter to more accurately reflect its purpose within wait_subprocess, making it obvious that passing a different value than what was passed to create_pipe_*'s null_stderr is acceptable. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpkVRsACgkQ84KuGfSFAYCLugCdHFDOoWwf7OSWPuTnzxrI8kI6 NjUAoJ3w2w+X94bPYNb2eBukBIVWZCMs =BvUu -END PGP SIGNATURE-
Re: Cygwin 1.7 wide char functions
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 7/18/2009 10:41 AM: >>> checking whether mbrtowc has a correct return value... no >> This is the first one the Cygwin developers should take care of. They are looking into it. > The setlocale function [1] installs function pointers to functions > __eucjp_wctomb and __eucjp_mbtowc, which are implemented in [2] and [3]. > [2] http://cygwin.com/cgi-bin/cvsweb.cgi/src/newlib/libc/stdlib/wctomb_r.c?rev=1.14&content-type=text/x-cvsweb-markup&cvsroot=src > [3] > http://cygwin.com/cgi-bin/cvsweb.cgi/src/newlib/libc/stdlib/mbtowc_r.c?rev=1.13&content-type=text/x-cvsweb-markup&cvsroot=src No, those are the newlib implementations; cygwin provides override implementations that are consistent with cygwin's difference in encoding values. http://cygwin.com/ml/cygwin/2009-07/msg00711.html - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpkWXMACgkQ84KuGfSFAYAz8wCgplK57sPu62IU0WSW/bff6A3q 41YAoK13DRwISG/5QZAwK+g7oKKADHhf =WCBe -END PGP SIGNATURE-
Re: Cygwin 1.7 wide char functions
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Eric Blake on 7/20/2009 5:48 AM: > According to Bruno Haible on 7/18/2009 10:41 AM: checking whether mbrtowc has a correct return value... no >>> This is the first one the Cygwin developers should take care of. > > They are looking into it. The verdict - the eucJP (and several other) charset is supported only if you have installed it in your version of Windows (newer versions have it installed automatically). On systems where they are installed, the mbrtowc function works. http://cygwin.com/ml/cygwin/2009-07/msg00721.html But this means that if a program is built on a system with the code page, but then run on a system without, that tests trying to use that locale will fail - maybe it is just a matter of teaching the tests to skip instead of crash if it is detected that the code page is lacking. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpkZSUACgkQ84KuGfSFAYD5BACfc+OX93PGFka19YDt2Sjgfcbu u6kAoKfgr7E+oC3bkEFv0ueauBugCj6J =wHU7 -END PGP SIGNATURE-
Re: Help with create_pipe_bidi
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 According to Bruno Haible on 7/19/2009 4:53 AM: > I've committed this. The test now passes on mingw (and of course also on > Linux and Cygwin). I'm committing this followup, which fixes a comment rendered stale by your previous commit, adds the same cautionary comment that you are proposing for pipe-filter, and enhances the testsuite to 1) attempt a larger read in the child while rearranging calls in the parent to avoid deadlock in that case, and 2) always report ASSERT failures to the original stderr, regardless of what the test did to fd 2. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpkZzAACgkQ84KuGfSFAYDmvwCdFSqIbhXEZ9mGs4BRYPZAfEoZ LpgAoIxPrrtzhqv+okhKjjJB3eYYqfK4 =nMVB -END PGP SIGNATURE- From deb58d481fc8dc345e463002c7087a08a4f1d5f0 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 20 Jul 2009 06:41:01 -0600 Subject: [PATCH] test-pipe: make a bit more robust. * tests/test-pipe.c (myerr): Allow error messages regardless of what we do to stderr. (test_pipe): Rearrange to avoid deadlock. (child_main): Try a larger read, to ensure we avoided deadlock. * lib/pipe.c (create_pipe) [_WIN32]: Fix comment. * lib/pipe.h (create_pipe_bidi): Document potential for deadlock if misused. Signed-off-by: Eric Blake --- ChangeLog | 11 +++ lib/pipe.c|4 +--- lib/pipe.h| 12 tests/test-pipe.c | 39 --- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9c78852..ea5f5fe 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2009-07-20 Eric Blake + + test-pipe: make a bit more robust. + * tests/test-pipe.c (myerr): Allow error messages regardless of + what we do to stderr. + (test_pipe): Rearrange to avoid deadlock. + (child_main): Try a larger read, to ensure we avoided deadlock. + * lib/pipe.c (create_pipe) [_WIN32]: Fix comment. + * lib/pipe.h (create_pipe_bidi): Document potential for deadlock + if misused. + 2009-07-19 Jim Meyering fts: avoid false-positive cycle-detection diff --git a/lib/pipe.c b/lib/pipe.c index 5d91590..daf7f7f 100644 --- a/lib/pipe.c +++ b/lib/pipe.c @@ -185,9 +185,7 @@ create_pipe (const char *progname, && close (stdoutfd) >= 0) /* The child process doesn't inherit ifd[0], ifd[1], ofd[0], ofd[1], but it inherits all open()ed or dup2()ed file handles (which is what - we want in the case of STD*_FILENO) and also orig_stdin, - orig_stdout, orig_stderr (which is not explicitly wanted but - harmless). */ + we want in the case of STD*_FILENO). */ /* Use spawnvpe and pass the environment explicitly. This is needed if the program has modified the environment using putenv() or [un]setenv(). On Windows, programs have two environments, one in the "environment diff --git a/lib/pipe.h b/lib/pipe.h index 03f2c32..082f687 100644 --- a/lib/pipe.h +++ b/lib/pipe.h @@ -109,6 +109,18 @@ extern pid_t create_pipe_in (const char *progname, * * Note: When writing to a child process, it is useful to ignore the SIGPIPE * signal and the EPIPE error code. + * + * Note: The parent process must be careful to avoid deadlock. + * 1) If you write more than PIPE_MAX bytes or, more generally, if you write + *more bytes than the subprocess can handle at once, the subprocess + *may write its data and wait on you to read it, but you are currently + *busy writing. + * 2) When you don't know ahead of time how many bytes the subprocess + *will produce, the usual technique of calling read (fd, buf, BUFSIZ) + *with a fixed BUFSIZ will, on Linux 2.2.17 and on BSD systems, cause + *the read() call to block until *all* of the buffer has been filled. + *But the subprocess cannot produce more data until you gave it more + *input. But you are currently busy reading from it. */ extern pid_t create_pipe_bidi (const char *progname, const char *prog_path, char **prog_argv, diff --git a/tests/test-pipe.c b/tests/test-pipe.c index 023f403..e28fae7 100644 --- a/tests/test-pipe.c +++ b/tests/test-pipe.c @@ -34,15 +34,19 @@ #include /* Depending on arguments, this test intentionally closes stderr or - starts life with stderr closed. So, the error messages might not - always print, but at least the exit status will be reliable. */ + starts life with stderr closed. So, we arrange to have fd 10 + (outside the range of interesting fd's during the test) set up to + duplicate the original stderr. */ + +static FILE *myerr; + #define ASSERT(expr) \ do
[PATCH] lib/sha1.h: wrap declarations in extern "C" scope when included from C++
>From 640f2ed9570eef3189e43ec7550b32776fdebd0f Mon Sep 17 00:00:00 2001
From: Peter Simons
Date: Sun, 19 Jul 2009 18:25:44 +0200
Subject: [PATCH] lib/sha1.h: wrap declarations in extern "C" scope when
included from C++
---
lib/sha1.h |8
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/lib/sha1.h b/lib/sha1.h
index 4ffda7a..b5f725b 100644
--- a/lib/sha1.h
+++ b/lib/sha1.h
@@ -23,6 +23,10 @@
# include
# include
+#ifdef __cplusplus
+extern "C" {
+#endif
+
#define SHA1_DIGEST_SIZE 20
/* Structure to save state of computation between the single steps. */
@@ -81,4 +85,8 @@ extern int sha1_stream (FILE *stream, void *resblock);
digest. */
extern void *sha1_buffer (const char *buffer, size_t len, void *resblock);
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
#endif
--
1.6.3.3
Re: Cygwin 1.7 wide char functions
Eric Blake wrote: > The verdict - the eucJP (and several other) charset is supported only if > you have installed it in your version of Windows (newer versions have it > installed automatically). On systems where they are installed, the > mbrtowc function works. Fixing setlocale() to return info about whether the locale is actually supported is perfect. With this, gnulib's autoconf tests probably don't need to be modified. Bruno
Re: Help with create_pipe_bidi
Eric Blake wrote: > ... rearranging calls in the parent to avoid deadlock ... Thanks! This possible deadlock was so easy to overlook. Bruno
Re: Cygwin 1.7 wide char functions
Bruno Haible clisp.org> writes: > > The verdict - the eucJP (and several other) charset is supported only if > > you have installed it in your version of Windows (newer versions have it > > installed automatically). On systems where they are installed, the > > mbrtowc function works. > > Fixing setlocale() to return info about whether the locale is actually > supported is perfect. With this, gnulib's autoconf tests probably don't > need to be modified. Confirmed: Before today, with the tables uninstalled I saw this, with subsequent test failures during 'make check': checking for a traditional japanese locale... (cached) ja_JP.eucJP checking whether mbrtowc has a correct return value... no With today's patch to cygwin [1], and the tables uninstalled, I see this, with previous failures now treated as skip during 'make check': checking for a traditional japanese locale... (cached) none checking whether mbrtowc has a correct return value... guessing yes And with the tables installed (with or without today's cygwin patch), I see this, with new 'pass' during 'make check: checking for a traditional japanese locale... (cached) ja_JP.eucJP checking whether mbrtowc has a correct return value... yes [1] http://cygwin.com/ml/cygwin-cvs/2009-q3/msg00058.html -- Eric Blake
Re: overly zealous git hook
Bruno Haible wrote: > Hi Jim, > > The git server at savannah now rejects the normal form of module > descriptions. I got this error message: > > modules/pipe-filter:34: ends with blank lines. > error: hooks/update exited with error code 2 > error: hook declined to update refs/heads/master > > I understand that for .c, .h, .texi and many other files, a blank line > at the end of the file is an indication of sloppiness. Not so for > gnulib module description files. > > A gnulib modules description formally consists of sections. Every section > consists of one or more lines. The first line of a section is a special > keyword, followed by a colon (such as 'Makefile.am:'). Then comes a number > of content lines. At the end comes - by convention - a blank line. The > sections are in arbitrary order. Hi Bruno, My modules files adhere to the no-trailing-blank-line convention. > Because a blank line actually belongs to the section that started before > it (see how it's parsed in gnulib-tool: variable sed_extract_prog), it's > not a separator. Therefore it's perfectly normal that module descriptions > end in 1 blank line. > > Likewise for ChangeLog files: They consist of entries, each ending with a > blank line. Therefore, normally, a ChangeLog file ends with a blank line. > > Can you please exempt the files under modules/ and the ChangeLog files > from this hooks/update check? Actually, I see trailing blank lines in any text file as an opportunity for unexpected merge conflicts, as one person adds or removes one of those oft-unnoticed lines, and someone else makes a conflicting change. Of course, being only at the end of the file, it's not likely to cause trouble, but I've preferred to remove them from all of my files for years. FYI, this change in behavior is the result of savannah upgrading to the latest version of git, whose git diff --check tests for that. Since I prefer that the hook continue to apply to all files for which I have a say, perhaps you'd like to itemize the files for which you want to allow trailing blank lines? Jim
Re: Help with create_pipe_bidi
Eric Blake byu.net> writes: > always report ASSERT failures to the original stderr, > regardless of what the test did to fd 2. Well, it would work if mingw's dup2 weren't broken. On mingw, dup2 always returns 0 for success, rather than the fd just opened. > + /* We might close fd 2 later, so save it in fd 10. */ > + if (dup2 (STDERR_FILENO, 10) != 10 > + || (myerr = fdopen (10, "w")) == NULL) > +return 2; > + return parent_main (argc, argv); > } So I guess I'll start the process of writing a dup2 replacement module; meanwhile, we can just use the hack of checking that the dup2 result in this test was non-negative. -- Eric Blake
Re: dup2 on mingw
Eric Blake wrote: > On mingw, dup2 always returns 0 for success, rather than the fd just opened. > So I guess I'll start the process of writing a dup2 replacement module; Yes, that's definitely a portability pitfall that warrants a wrapper module. Bruno
Re: overly zealous git hook
Hi Jim, > Actually, I see trailing blank lines in any text file > as an opportunity for unexpected merge conflicts, as > one person adds or removes one of those oft-unnoticed lines, > and someone else makes a conflicting change. Conflicts generally occur at spots that are heavily modified, such as the end of log files or the start of ChangeLog files. For modules/* files, the last section is the 'Maintainer' section, which rarely changes. Likewise for the end of ChangeLog files: it does not change at all, usually. > FYI, this change in behavior is the result of savannah upgrading to > the latest version of git, whose git diff --check tests for that. Ah, understood. > Since I prefer that the hook continue to apply to all files > for which I have a say, perhaps you'd like to itemize the files > for which you want to allow trailing blank lines? Yes. These should be: modules/**/* and **/ChangeLog*. I believe it can be done by adding to .gitattributes a line ChangeLog* whitespace=- and to modules/.gitattributes a line * whitespace=- where is the whitespace rule that triggers the error. Bruno
