Pádraig Brady <p...@draigbrady.com> wrote: > I've started to work on the stdbuf util referenced in this thread: > http://lists.gnu.org/archive/html/bug-coreutils/2008-11/threads.html#00134 > I've a few questions...
Hi Pádraig, Thanks for writing/posting that. > The main one is should I introduce a dependency on libtool or use > something else. I'll be installing an unversioned shared lib (plugin) My initial reaction is to avoid it. > on ELF systems, so I'm not sure the libtool dependency is desired? > Perhaps it's needed considering cygwin supports LD_PRELOAD? We'll see. > The stdbuf util is built only on ELF systems. Is my check for such > systems in configure.ac appropriate? Nice and minimal. But I haven't researched it at all. > (Note libstdbuf.so is built unconditionally for the moment, > and is not installed. But the lib will be used if it's in the > same directory as the stdbuf util). I'd rather avoid installing the .so alongside the other binaries. > I've added code to stdbuf to try and specify the full path > to the lib to LD_PRELOAD, rather than letting the dynamic > linker search for it. This is so as to remove the need to > add our lib install dir to the system search path, and > also simplifies testing as we can run stdbuf through the path > without having to setup LD_LIBRARY_PATH. Any gotchas with doing this? I like not having to modify LD_LIBRARY_PATH. We might want to substitute one directory name at build time, so pre-installation tests work, and then to use a different one in the installed binary. > I've currently split the util and shared lib into stdbuf.c > and libstdbuf.c. It might be better to just have a stdbuf.c > and build both binary and shared lib from that? Yes, that sounds like it might be better -- but realize I've barely glanced through the actual code. Thanks. Here's an incremental so that "make syntax-check" passes. Additional syntax changes you'll need, eventually: - filter thought indent so formatting (e.g. space-before-paren and around arithmetic operators) is consistent - convert the //-style comments to /*...*/ >From 3ca87ad09f130e4d128e4a9681156eda8917d7d2 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@redhat.com> Date: Tue, 3 Feb 2009 10:07:37 +0100 Subject: [PATCH] mostly nits * src/Makefile.am: don't make new rule the first (default) * cfg.mk (sc_system_h_headers): Use VC_LIST_EXCEPT rather than VC_LIST, so we can add an exception, if needed. * .x-sc_system_h_headers: New file. Exempt libstdbuf.c. * po/POTFILES.in: * src/libstdbuf.c: * src/stdbuf.c: --- .x-sc_system_h_headers | 3 +++ AUTHORS | 1 + cfg.mk | 3 +-- po/POTFILES.in | 1 + src/Makefile.am | 7 ++++--- src/libstdbuf.c | 1 + src/stdbuf.c | 20 ++++++++++---------- 7 files changed, 21 insertions(+), 15 deletions(-) create mode 100644 .x-sc_system_h_headers diff --git a/.x-sc_system_h_headers b/.x-sc_system_h_headers new file mode 100644 index 0000000..14e020f --- /dev/null +++ b/.x-sc_system_h_headers @@ -0,0 +1,3 @@ +^src/libstdbuf\.c$ +^src/system\.h$ +^src/copy\.h$ diff --git a/AUTHORS b/AUTHORS index fa3c029..7095db0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -76,6 +76,7 @@ sleep: Jim Meyering, Paul Eggert sort: Mike Haertel, Paul Eggert split: Torbjörn Granlund, Richard M. Stallman stat: Michael Meskes +stdbuf: Pádraig Brady stty: David MacKenzie su: David MacKenzie sum: Kayvan Aghaiepour, David MacKenzie diff --git a/cfg.mk b/cfg.mk index 0e42042..2f356c2 100644 --- a/cfg.mk +++ b/cfg.mk @@ -151,8 +151,7 @@ sc_system_h_headers: .re-list @if test -f $(srcdir)/src/system.h; then \ trap 'rc=$$?; rm -f .re-list; exit $$rc' 0 1 2 3 15; \ grep -nE -f .re-list \ - $$($(VC_LIST) src | \ - grep -Ev '((copy|system)\.h|parse-gram\.c)$$') \ + $$($(VC_LIST_EXCEPT) | grep '^src/') \ && { echo '$(ME): the above are already included via system.h'\ 1>&2; exit 1; } || :; \ fi diff --git a/po/POTFILES.in b/po/POTFILES.in index 6c291cc..921da4f 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -109,6 +109,7 @@ src/sleep.c src/sort.c src/split.c src/stat.c +src/stdbuf.c src/stty.c src/su.c src/sum.c diff --git a/src/Makefile.am b/src/Makefile.am index ca12f9a..c0e7162 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -44,9 +44,6 @@ EXTRA_PROGRAMS = \ test timeout true truncate tty whoami yes \ base64 -libstdbuf.so: libstdbuf.c - gcc -fPIC -shared -o libstdbuf.so libstdbuf.c - bin_PROGRAMS = $(OPTIONAL_BIN_PROGS) noinst_PROGRAMS = setuidgid getlimits libstdbuf.so @@ -170,6 +167,7 @@ du_LDADD += $(LIBICONV) getlimits_LDADD += $(LIBICONV) ptx_LDADD += $(LIBICONV) split_LDADD += $(LIBICONV) +stdbuf_LDADD += $(LIBICONV) timeout_LDADD += $(LIBICONV) truncate_LDADD += $(LIBICONV) @@ -179,6 +177,9 @@ who_LDADD = $(LDADD) $(GETADDRINFO_LIB) $(PROGRAMS): ../lib/libcoreutils.a +libstdbuf.so: libstdbuf.c + gcc $(AM_CPPFLAGS) -fPIC -shared -o libstdbuf.so libstdbuf.c + # Get the release year from ../lib/version-etc.c. RELEASE_YEAR = \ `sed -n '/.*COPYRIGHT_YEAR = \([0-9][0-9][0-9][0-9]\) };/s//\1/p' \ diff --git a/src/libstdbuf.c b/src/libstdbuf.c index 34e0b58..194b624 100644 --- a/src/libstdbuf.c +++ b/src/libstdbuf.c @@ -1,5 +1,6 @@ /* Assume all input validation has been done by stdbuf. */ +#include <config.h> #include <stdio.h> //#include <stdio_ext.h> #include <stdlib.h> diff --git a/src/stdbuf.c b/src/stdbuf.c index 8472412..100c4a2 100644 --- a/src/stdbuf.c +++ b/src/stdbuf.c @@ -20,7 +20,6 @@ * However tests seem to be run unconditionally? */ #include <config.h> -#include <configmake.h> #include <stdio.h> #include <getopt.h> #include <sys/types.h> @@ -94,9 +93,9 @@ Run COMMAND, with modified buffering operations for its standard streams.\n\ Mandatory arguments to long options are mandatory for short options too.\n\ "), stdout); fputs (_("\ - -i, --input=MODE Setup standard input stream buffering\n\ - -o, --output=MODE Setup standard output stream buffering\n\ - -e, --error=MODE Setup standard error stream buffering\n\ + -i, --input=MODE Setup standard input stream buffering\n\ + -o, --output=MODE Setup standard output stream buffering\n\ + -e, --error=MODE Setup standard error stream buffering\n\ "), stdout); fputs (HELP_OPTION_DESCRIPTION, stdout); fputs (VERSION_OPTION_DESCRIPTION, stdout); @@ -156,14 +155,14 @@ set_LD_PRELOAD(void) //However we want the lookup done for the exec'd command not stdbuf. // //Not linked against, but is possibly searched for, so add to LIBDIR rather than LIBEXECDIR - char* search_path[] = { + char const* const search_path[] = { program_path, PKGLIBDIR, "", /* sys default */ NULL }; - char** path = search_path; + char const* const* path = search_path; char* libstdbuf; do @@ -206,9 +205,10 @@ set_LD_PRELOAD(void) using $PATH. In the latter case to get the path we can: search getenv("PATH"), readlink("/prof/self/exe"), getenv("_"), dladdr(), pstat_getpathname(), etc. */ -void set_program_path(const char* arg) +static void +set_program_path (const char* arg) { - if (strchr(arg, '/')) /* Use absolute or relative paths directly. */ + if (strchr (arg, '/')) /* Use absolute or relative paths directly. */ { program_path = dir_name (arg); } @@ -231,7 +231,7 @@ void set_program_path(const char* arg) int req = snprintf(tmppath, sizeof(tmppath), "%s/%s", dir, arg); if (req >= sizeof(tmppath)) { - error (0, 0, _("Path truncated when looking for %s"), + error (0, 0, _("path truncated when looking for %s"), quote(arg)); } else if (access(tmppath, X_OK) == 0) @@ -284,7 +284,7 @@ main (int argc, char **argv) /* -oL will be by far the most common use of this utility, * but one could easily think -iL might have the same affect, * so disallow it as it could be confusing. */ - error (0, 0, _("Line buffering stdin is meaninglesss")); + error (0, 0, _("line buffering stdin is meaninglesss")); usage (EXIT_CANCELED); } -- 1.6.1.2.443.g0d7c2 _______________________________________________ Bug-coreutils mailing list Bug-coreutils@gnu.org http://lists.gnu.org/mailman/listinfo/bug-coreutils