Re: cwrapper generates long strings.

2010-10-04 Thread Ralf Wildenhues
* Peter Rosin wrote on Sun, Oct 03, 2010 at 08:28:47PM CEST:
 Den 2010-10-03 09:01 skrev Ralf Wildenhues:
  +for i in 25 50 75 100 125 150 175 200 225 250
  +do
  +  PATH=$PATH$PATH_SEPARATOR/somewhere-that-eksists-not

  Does $LIBTOOL or the autotest machinery ever intentionally try to run
  commands that won't exist in $PATH within this shell?
 
 I don't think so, and it is a really hard question to answer too.

Indeed.

I'm kinda wondering whether we should at least delimit our use of
nonexistent files and directories to a common subtree, like below
/nonexistent or so.  I realize we're getting near bike shedding
issues though, so how about we cross that bridge when we get to it,
and leave your patch as is for now.

   If so, then we
  might get the odd bug report from security-hardened distributions that
  complain about read or execute accessses to non-allowed parts of the
  file system.
 
 Using $PATH$PATH_SEPARATOR$PATH seemed like a much quicker way
 to make the length explode so I didn't like that.

OK.

  This length doesn't yet trigger the compiler string literal length
  limit; not sure whether it should?
 
 That's not reliable anyway as most compilers support so long strings
 that it's hard to tickle it.

FWIW, that is not necessary.  It would be sufficient if it were tickled
with the one compiler in question.

 On a tangent, another bug is that linking
 doesn't fail (libtool returns zero) when building the cwrapper fails.
 I think that's a separate issue from this one, which is why I haven't
 mixed them up previously.

OK, good.

 Another nit in that area is that there are
 multiple levels of $opt_dry_run || { which seems superfluous, but
 that might be intentional in order to keep the code copy-paste-safe?

Not sure.  I don't have much sympathy for copy-paste-safety, because
I dislike the kludge that copy-paste is.  Functions should be used
instead.  So yes, I guess a cleanup is a good idea, after ensuring that
the code really works fine with the outer opt_dry_run enclosing.

  Do we have to cater to the case where the environment is very large
  already?  I'm not sure, we might want to deal with it when crossing
  that bridge only, if it's hard to know off-hand.
 
 Are your three above paragraphs nits and part of what I need to
 address, or just notes for the future?

They started out as nits, but I guess they've become notes by now.
So go ahead with your patch, please.

  +# try to make sure the test is relevant
  +AT_CHECK([grep ' *fputs' $objdir/lt-usea.c  /dev/null])
  
  Rather than redirecting stdout, put [ignore] in the third argument.
 
 Certainly possible, but I didn't think anyone would be interested in a
 couple of hundred lines of boilerplate in the log.  I don't really care
 though, so if you still think [ignore] is a good idea, then ok.

Ah yes.  Autoconf 2.64 provides stdout-nolog for this, but I guess you
can keep the code the way it is, for compatibility.

Thanks,
Ralf



Re: cwrapper generates long strings.

2010-10-04 Thread Peter Rosin
Den 2010-10-04 07:00 skrev Ralf Wildenhues:
 * Peter Rosin wrote on Sun, Oct 03, 2010 at 08:28:47PM CEST:
 Den 2010-10-03 09:01 skrev Ralf Wildenhues:
 +for i in 25 50 75 100 125 150 175 200 225 250
 +do
 +  PATH=$PATH$PATH_SEPARATOR/somewhere-that-eksists-not
 
 Does $LIBTOOL or the autotest machinery ever intentionally try to run
 commands that won't exist in $PATH within this shell?

 I don't think so, and it is a really hard question to answer too.
 
 Indeed.
 
 I'm kinda wondering whether we should at least delimit our use of
 nonexistent files and directories to a common subtree, like below
 /nonexistent or so.  I realize we're getting near bike shedding
 issues though, so how about we cross that bridge when we get to it,
 and leave your patch as is for now.
 
  If so, then we
 might get the odd bug report from security-hardened distributions that
 complain about read or execute accessses to non-allowed parts of the
 file system.

 Using $PATH$PATH_SEPARATOR$PATH seemed like a much quicker way
 to make the length explode so I didn't like that.
 
 OK.

I wrote a loop appending the first PATH component until the length
exceeds the limit.  The longest possible PATH should be 499 characters
this way, which seems OK to me, and it should have no wild components.

 This length doesn't yet trigger the compiler string literal length
 limit; not sure whether it should?

 That's not reliable anyway as most compilers support so long strings
 that it's hard to tickle it.
 
 FWIW, that is not necessary.  It would be sufficient if it were tickled
 with the one compiler in question.

Since the compiler limit in this case is as large as 2048 (whoooa), which
is about the same as you quoted for grep, I decided to not do that.

 On a tangent, another bug is that linking
 doesn't fail (libtool returns zero) when building the cwrapper fails.
 I think that's a separate issue from this one, which is why I haven't
 mixed them up previously.
 
 OK, good.
 
 Another nit in that area is that there are
 multiple levels of $opt_dry_run || { which seems superfluous, but
 that might be intentional in order to keep the code copy-paste-safe?
 
 Not sure.  I don't have much sympathy for copy-paste-safety, because
 I dislike the kludge that copy-paste is.  Functions should be used
 instead.  So yes, I guess a cleanup is a good idea, after ensuring that
 the code really works fine with the outer opt_dry_run enclosing.

Ok.

 Do we have to cater to the case where the environment is very large
 already?  I'm not sure, we might want to deal with it when crossing
 that bridge only, if it's hard to know off-hand.

 Are your three above paragraphs nits and part of what I need to
 address, or just notes for the future?
 
 They started out as nits, but I guess they've become notes by now.
 So go ahead with your patch, please.

Holding it up for one more iteration.

 +# try to make sure the test is relevant
 +AT_CHECK([grep ' *fputs' $objdir/lt-usea.c  /dev/null])

 Rather than redirecting stdout, put [ignore] in the third argument.

 Certainly possible, but I didn't think anyone would be interested in a
 couple of hundred lines of boilerplate in the log.  I don't really care
 though, so if you still think [ignore] is a good idea, then ok.
 
 Ah yes.  Autoconf 2.64 provides stdout-nolog for this, but I guess you
 can keep the code the way it is, for compatibility.

Ok.

Cheers,
Peter

From 0cdd5a00c698cc47e4c7d1af377b7fb5090a417b Mon Sep 17 00:00:00 2001
From: Peter Rosin p...@lysator.liu.se
Date: Mon, 4 Oct 2010 09:40:45 +0200
Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.

* libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
the wrapper script contains long lines, split them for
readability and to conform with C standards.
* tests/cwrapper.at (cwrapper string length): New test, making
sure we don't regress.

Signed-off-by: Peter Rosin p...@lysator.liu.se
---
 ChangeLog  |9 ++
 libltdl/config/ltmain.m4sh |   12 ++--
 tests/cwrapper.at  |   61 
 3 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a7aa489..e45bfe8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-10-04  Peter Rosin  p...@lysator.liu.se
+
+   cwrapper: split long lines when dumping the wrapper script.
+   * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
+   the wrapper script contains long lines, split them for
+   readability and to conform with C standards.
+   * tests/cwrapper.at (cwrapper string length): New test, making
+   sure we don't regress.
+
 2010-09-27  Peter Rosin  p...@lysator.liu.se
 
tests: check if sys_lib_search_path_spec works on MSVC.
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 0418007..1078e75 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -4268,9 +4268,15 @@ void lt_dump_script (FILE* f)
 

Re: cwrapper generates long strings.

2010-10-04 Thread Ralf Wildenhues
* Peter Rosin wrote on Mon, Oct 04, 2010 at 10:02:15AM CEST:
 
 I wrote a loop appending the first PATH component until the length
 exceeds the limit.  The longest possible PATH should be 499 characters
 this way, which seems OK to me, and it should have no wild components.

Cool.

 Den 2010-10-04 07:00 skrev Ralf Wildenhues:
  * Peter Rosin wrote on Sun, Oct 03, 2010 at 08:28:47PM CEST:
  This length doesn't yet trigger the compiler string literal length
  limit; not sure whether it should?
 
  That's not reliable anyway as most compilers support so long strings
  that it's hard to tickle it.
  
  FWIW, that is not necessary.  It would be sufficient if it were tickled
  with the one compiler in question.
 
 Since the compiler limit in this case is as large as 2048 (whoooa), which
 is about the same as you quoted for grep, I decided to not do that.

Good point.

The updated patch is fine.

Thanks,
Ralf

 Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.
 
 * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
 the wrapper script contains long lines, split them for
 readability and to conform with C standards.
 * tests/cwrapper.at (cwrapper string length): New test, making
 sure we don't regress.



Re: cwrapper generates long strings.

2010-10-03 Thread Ralf Wildenhues
Hi Peter,

* Peter Rosin wrote on Sat, Oct 02, 2010 at 11:33:02PM CEST:
 Den 2010-10-02 13:53 skrev Ralf Wildenhues:
  Yes.  Well, we might get the odd report about the Cygwin non-binmode
  mount where the CR NL messes up things, but otherwise, it should work.
 
 If you are on a text mount, it should fixup CR NL issues.  If you have
 CR NL files on a binary mount you deserve to suffer.  So, a non-issue.

Cool.  Thanks.

 I used 250 at the limit in the test as the 79 characters from the string
 splitter in ltmain might be doubled due to C string escapes and then I
 added some extra margin for quotes and the , f); trailer.  Still below
 255 which might be an arbitrary limit in some grep implementations.

You can assume close to 2K as minimum limit for grep.
(Hmm, wonder why we didn't ever write down the value in autoconf.texi)

The patch is OK with nits addressed.

 --- a/tests/cwrapper.at
 +++ b/tests/cwrapper.at
 @@ -134,3 +134,53 @@ done
  
  AT_CLEANUP
  
 +
 +AT_SETUP([cwrapper string length])
 +
 +eval `$LIBTOOL --config | $EGREP '^(objdir)='`
 +
 +AT_DATA([liba.c],
 +[[int liba_func1 (int arg)
 +{
 +  return arg + 1;
 +}
 +]])
 +AT_DATA([usea.c],
 +[[extern int liba_func1 (int arg);
 +int main (void)
 +{
 +  int a = 2;
 +  int b = liba_func1 (a);
 +  if (b == 3) return 0;
 +  return 1;
 +}
 +]])
 +
 +AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
 +  [], [ignore], [ignore])
 +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined ]dnl
 +  [-o liba.la -rpath /foo liba.lo],
 +  [], [ignore], [ignore])
 +AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
 +  [], [ignore], [ignore])
 +
 +# make sure PATH is at least 250 chars, should force line breaks in lt-usea.c
 +for i in 25 50 75 100 125 150 175 200 225 250
 +do
 +  PATH=$PATH$PATH_SEPARATOR/somewhere-that-eksists-not

Intended typo 'exists'?  ;-)

Does $LIBTOOL or the autotest machinery ever intentionally try to run
commands that won't exist in $PATH within this shell?  If so, then we
might get the odd bug report from security-hardened distributions that
complain about read or execute accessses to non-allowed parts of the
file system.

This length doesn't yet trigger the compiler string literal length
limit; not sure whether it should?

Do we have to cater to the case where the environment is very large
already?  I'm not sure, we might want to deal with it when crossing
that bridge only, if it's hard to know off-hand.

 +done
 +export PATH
 +
 +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-fast-install ]dnl
 +  [-o usea$EXEEXT usea.$OBJEXT liba.la],
 +  [], [ignore], [ignore])
 +
 +# skip if no cwrapper is generated
 +AT_CHECK([test -f $objdir/lt-usea.c || exit 77])
 +
 +# try to make sure the test is relevant
 +AT_CHECK([grep ' *fputs' $objdir/lt-usea.c  /dev/null])

Rather than redirecting stdout, put [ignore] in the third argument.

 +# check for no overly long fputs
 +AT_CHECK([grep ' *fputs.\{250\}' $objdir/lt-usea.c], [1])
 +
 +AT_CLEANUP

Cheers,
Ralf



Re: cwrapper generates long strings.

2010-10-03 Thread Peter Rosin
Den 2010-10-03 09:01 skrev Ralf Wildenhues:
 I used 250 at the limit in the test as the 79 characters from the string
 splitter in ltmain might be doubled due to C string escapes and then I
 added some extra margin for quotes and the , f); trailer.  Still below
 255 which might be an arbitrary limit in some grep implementations.
 
 You can assume close to 2K as minimum limit for grep.
 (Hmm, wonder why we didn't ever write down the value in autoconf.texi)
 
 The patch is OK with nits addressed.
 
 --- a/tests/cwrapper.at
 +++ b/tests/cwrapper.at
 @@ -134,3 +134,53 @@ done
  
  AT_CLEANUP
  
 +
 +AT_SETUP([cwrapper string length])
 +
 +eval `$LIBTOOL --config | $EGREP '^(objdir)='`
 +
 +AT_DATA([liba.c],
 +[[int liba_func1 (int arg)
 +{
 +  return arg + 1;
 +}
 +]])
 +AT_DATA([usea.c],
 +[[extern int liba_func1 (int arg);
 +int main (void)
 +{
 +  int a = 2;
 +  int b = liba_func1 (a);
 +  if (b == 3) return 0;
 +  return 1;
 +}
 +]])
 +
 +AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
 + [], [ignore], [ignore])
 +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined ]dnl
 + [-o liba.la -rpath /foo liba.lo],
 + [], [ignore], [ignore])
 +AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
 + [], [ignore], [ignore])
 +
 +# make sure PATH is at least 250 chars, should force line breaks in 
 lt-usea.c
 +for i in 25 50 75 100 125 150 175 200 225 250
 +do
 +  PATH=$PATH$PATH_SEPARATOR/somewhere-that-eksists-not
 
 Intended typo 'exists'?  ;-)

Yes :-)

 Does $LIBTOOL or the autotest machinery ever intentionally try to run
 commands that won't exist in $PATH within this shell?

I don't think so, and it is a really hard question to answer too.

  If so, then we
 might get the odd bug report from security-hardened distributions that
 complain about read or execute accessses to non-allowed parts of the
 file system.

Using $PATH$PATH_SEPARATOR$PATH seemed like a much quicker way
to make the length explode so I didn't like that.

 This length doesn't yet trigger the compiler string literal length
 limit; not sure whether it should?

That's not reliable anyway as most compilers support so long strings
that it's hard to tickle it. On a tangent, another bug is that linking
doesn't fail (libtool returns zero) when building the cwrapper fails.
I think that's a separate issue from this one, which is why I haven't
mixed them up previously.  Another nit in that area is that there are
multiple levels of $opt_dry_run || { which seems superfluous, but
that might be intentional in order to keep the code copy-paste-safe?

 Do we have to cater to the case where the environment is very large
 already?  I'm not sure, we might want to deal with it when crossing
 that bridge only, if it's hard to know off-hand.

Are your three above paragraphs nits and part of what I need to
address, or just notes for the future?

 +done
 +export PATH
 +
 +AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-fast-install ]dnl
 + [-o usea$EXEEXT usea.$OBJEXT liba.la],
 + [], [ignore], [ignore])
 +
 +# skip if no cwrapper is generated
 +AT_CHECK([test -f $objdir/lt-usea.c || exit 77])
 +
 +# try to make sure the test is relevant
 +AT_CHECK([grep ' *fputs' $objdir/lt-usea.c  /dev/null])
 
 Rather than redirecting stdout, put [ignore] in the third argument.

Certainly possible, but I didn't think anyone would be interested in a
couple of hundred lines of boilerplate in the log.  I don't really care
though, so if you still think [ignore] is a good idea, then ok.

 +# check for no overly long fputs
 +AT_CHECK([grep ' *fputs.\{250\}' $objdir/lt-usea.c], [1])
 +
 +AT_CLEANUP

Cheers,
Peter



Re: cwrapper generates long strings.

2010-10-02 Thread Ralf Wildenhues
Hi Peter,

* Peter Rosin wrote on Fri, Oct 01, 2010 at 11:22:54PM CEST:
 Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.
 
 * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
 the wrapper script contains long lines, split them for
 readability and to conform with C standards.

OK with me with nits addressed.  I see this as a fairly straightforward
way to work around the issue; we can still think about following Chuck's
proposal in due course even with this in place.

Nit 1: testsuite exposure.  Should be fairly straightforward to set
  PATH=$PATH$PATH_SEPARATOR$PATH

a couple of times until long enough (but not too long so that
environment plus command line length already go over the limit), then
build a library and a program linked against it, covering the path that
failed for you, no?

Thanks,
Ralf

 --- a/libltdl/config/ltmain.m4sh
 +++ b/libltdl/config/ltmain.m4sh
 @@ -4268,9 +4268,14 @@ void lt_dump_script (FILE* f)
  {
  EOF
   func_emit_wrapper yes |
 -  $SED -e 's/\([\\]\)/\\\1/g' \
 --e 's/^/  fputs (/' -e 's/$/\\n, f);/'
 -
 +   $SED -n -e '
 +s/^\(.\{79\}\)\(..*\)/\1\n\2/

\n is portable only in the regex part, but not in the replacement part.
For that you need backslash then literal newline.

 +h
 +s/\([\\]\)/\\\1/g
 +s/$/\\n/
 +s/\([^\n]*\).*/  fputs (\1, f);/p

Why not above, right after h, do s/\n.*// and then simplify this s
command?

 +g
 +D'
  cat EOF
  }
  EOF



Re: cwrapper generates long strings.

2010-10-02 Thread Peter Rosin
Den 2010-10-02 08:32 skrev Ralf Wildenhues:
 Hi Peter,
 
 * Peter Rosin wrote on Fri, Oct 01, 2010 at 11:22:54PM CEST:
 Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.

 * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
 the wrapper script contains long lines, split them for
 readability and to conform with C standards.
 
 OK with me with nits addressed.  I see this as a fairly straightforward
 way to work around the issue; we can still think about following Chuck's
 proposal in due course even with this in place.
 
 Nit 1: testsuite exposure.  Should be fairly straightforward to set
   PATH=$PATH$PATH_SEPARATOR$PATH
 
 a couple of times until long enough (but not too long so that
 environment plus command line length already go over the limit), then
 build a library and a program linked against it, covering the path that
 failed for you, no?

Should do it, but I can't fix that this weekend.  I guess I'll get to it
sometime next week.

 --- a/libltdl/config/ltmain.m4sh
 +++ b/libltdl/config/ltmain.m4sh
 @@ -4268,9 +4268,14 @@ void lt_dump_script (FILE* f)
  {
  EOF
  func_emit_wrapper yes |
 -  $SED -e 's/\([\\]\)/\\\1/g' \
 -   -e 's/^/  fputs (/' -e 's/$/\\n, f);/'
 -
 +  $SED -n -e '
 +s/^\(.\{79\}\)\(..*\)/\1\n\2/
 
 \n is portable only in the regex part, but not in the replacement part.
 For that you need backslash then literal newline.

Ok, so replacing with

s/^\(.\{79\}\)\(..*\)/\1\
\2/

is portable?  Easy enough, I'll fold it in with nit 1 and repost
before I push (if someone beats me to it and writes the test, feel
free to snarf the sed program and push if you do).

 +h
 +s/\([\\]\)/\\\1/g
 +s/$/\\n/
 +s/\([^\n]*\).*/  fputs (\1, f);/p
 
 Why not above, right after h, do s/\n.*// and then simplify this s
 command?
 
 +g
 +D'

Because then we no longer know if the C-string \n at the end of the
line (added by the 's/$/\\n/' statement) should be inserted or not.
The trick is to add it and then cut it out if the string was too long
(contains a literal newline from the s command).  We can only have
the \n for the final chunk, otherwise the output will be riddled
with too many newlines.

Cheers,
Peter



Re: cwrapper generates long strings.

2010-10-02 Thread Peter Rosin
Den 2010-10-02 13:53 skrev Ralf Wildenhues:
 * Peter Rosin wrote on Sat, Oct 02, 2010 at 01:42:02PM CEST:
 Den 2010-10-02 08:32 skrev Ralf Wildenhues:
 +$SED -n -e '
 +s/^\(.\{79\}\)\(..*\)/\1\n\2/

 \n is portable only in the regex part, but not in the replacement part.
 For that you need backslash then literal newline.

 Ok, so replacing with

 s/^\(.\{79\}\)\(..*\)/\1\
 \2/

 is portable?
 
 Yes.  Well, we might get the odd report about the Cygwin non-binmode
 mount where the CR NL messes up things, but otherwise, it should work.

If you are on a text mount, it should fixup CR NL issues.  If you have
CR NL files on a binary mount you deserve to suffer.  So, a non-issue.

I found unexpectedly found time early, so here's the updated patch
with a test case.

I used 250 at the limit in the test as the 79 characters from the string
splitter in ltmain might be doubled due to C string escapes and then I
added some extra margin for quotes and the , f); trailer.  Still below
255 which might be an arbitrary limit in some grep implementations.

Ok to push?

Cheers,
Peter

From 5e1b9944049b3956841f2af7e473f3e2504205d1 Mon Sep 17 00:00:00 2001
From: Peter Rosin p...@lysator.liu.se
Date: Sat, 2 Oct 2010 23:19:42 +0200
Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.

* libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
the wrapper script contains long lines, split them for
readability and to conform with C standards.
* tests/cwrapper.at (cwrapper string length): New test, making
sure we don't regress.

Signed-off-by: Peter Rosin p...@lysator.liu.se
---
 ChangeLog  |9 
 libltdl/config/ltmain.m4sh |   12 --
 tests/cwrapper.at  |   50 
 3 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a7aa489..db3585a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-10-02  Peter Rosin  p...@lysator.liu.se
+
+   cwrapper: split long lines when dumping the wrapper script.
+   * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
+   the wrapper script contains long lines, split them for
+   readability and to conform with C standards.
+   * tests/cwrapper.at (cwrapper string length): New test, making
+   sure we don't regress.
+
 2010-09-27  Peter Rosin  p...@lysator.liu.se
 
tests: check if sys_lib_search_path_spec works on MSVC.
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 0418007..1078e75 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -4268,9 +4268,15 @@ void lt_dump_script (FILE* f)
 {
 EOF
func_emit_wrapper yes |
-  $SED -e 's/\([\\]\)/\\\1/g' \
-  -e 's/^/  fputs (/' -e 's/$/\\n, f);/'
-
+ $SED -n -e '
+s/^\(.\{79\}\)\(..*\)/\1\
+\2/
+h
+s/\([\\]\)/\\\1/g
+s/$/\\n/
+s/\([^\n]*\).*/  fputs (\1, f);/p
+g
+D'
 cat EOF
 }
 EOF
diff --git a/tests/cwrapper.at b/tests/cwrapper.at
index 248c0c0..3c1b054 100644
--- a/tests/cwrapper.at
+++ b/tests/cwrapper.at
@@ -134,3 +134,53 @@ done
 
 AT_CLEANUP
 
+
+AT_SETUP([cwrapper string length])
+
+eval `$LIBTOOL --config | $EGREP '^(objdir)='`
+
+AT_DATA([liba.c],
+[[int liba_func1 (int arg)
+{
+  return arg + 1;
+}
+]])
+AT_DATA([usea.c],
+[[extern int liba_func1 (int arg);
+int main (void)
+{
+  int a = 2;
+  int b = liba_func1 (a);
+  if (b == 3) return 0;
+  return 1;
+}
+]])
+
+AT_CHECK([$LIBTOOL --mode=compile $CC $CPPFLAGS $CFLAGS -c liba.c],
+[], [ignore], [ignore])
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-undefined ]dnl
+[-o liba.la -rpath /foo liba.lo],
+[], [ignore], [ignore])
+AT_CHECK([$CC $CPPFLAGS $CFLAGS -c usea.c],
+[], [ignore], [ignore])
+
+# make sure PATH is at least 250 chars, should force line breaks in lt-usea.c
+for i in 25 50 75 100 125 150 175 200 225 250
+do
+  PATH=$PATH$PATH_SEPARATOR/somewhere-that-eksists-not
+done
+export PATH
+
+AT_CHECK([$LIBTOOL --mode=link $CC $CFLAGS $LDFLAGS -no-fast-install ]dnl
+[-o usea$EXEEXT usea.$OBJEXT liba.la],
+[], [ignore], [ignore])
+
+# skip if no cwrapper is generated
+AT_CHECK([test -f $objdir/lt-usea.c || exit 77])
+
+# try to make sure the test is relevant
+AT_CHECK([grep ' *fputs' $objdir/lt-usea.c  /dev/null])
+# check for no overly long fputs
+AT_CHECK([grep ' *fputs.\{250\}' $objdir/lt-usea.c], [1])
+
+AT_CLEANUP
-- 
1.7.1



Re: cwrapper generates long strings.

2010-10-01 Thread Peter Rosin
Hi Chuck,

Den 2010-10-01 16:03 skrev Charles Wilson:
 On 10/1/2010 7:28 AM, Peter Rosin wrote:
 I.e. I have this on line 865 in lt-main.c:

   fputs (relink_command=\(cd 
 /c/cygwin/home/peda/libtool/git/msvc/tests/testsuite.dir/112; 
 PATH=\\\/LOADS:/OF:/ENTRIES\\\; export PATH; 
 PATH=\\\/LOADS:/OF:/ENTRIES\\\; export PATH; 
 /c/cygwin/home/peda/automake/lib/compile cl -MD -Zi -EHsc -o @OUTPUT@ 
 .libs/main-static.obj  sub2/.libs/a.lib )\\n, f);

 In my case the string is 3400+ characters which is too much for MSVC 6,
 but this appears to not be really compiler specific, and I can easily
 imagine other compilers with other arbitrary (and possibly standardized)
 limits.
 
 c99 compilers must support const char* arrays of at least 4096.  Most
 compilers actually support much larger ones.
 
 c89 requires only support for up to 509.  See:
 http://lists.gnu.org/archive/html/libtool-patches/2008-04/msg00161.html
 http://lists.gnu.org/archive/html/libtool-patches/2009-01/msg4.html

I vaguely remember that, and rereading reveals that noone thought about
one individual line being too long (not publicly anyway).

 One thing that could be done is to only have the PATH once, but that is
 not a real fix.
 
 It's also not general.  One is explicitly setting PATH, the other is
 setting $shlibpath_var (which happens to be PATH on win32).
 
 Should we worry about my insane case?
 
 I don't think it is a high priority.  What we could do is implement a
 line emitter in ltmain.m4sh, for use by func_emit_cwrapper_exe.  Right
 now, it takes the output of func_emit_wrapper and puts fputs(\ and
 \n\ around each line.
 
 Instead, each line could be fed to a line-emitter function that chops
 the line into segments of = 500 chars.
 
 But...that's a lot of effort for very little gain.

Ok, I wouldn't say a lot of effort, and it was a bit of fun.

 I think a better approach would be to start asking, why do we still need
 the cwrapper to contain and emit the shell wrapper code in the first
 place?  Maybe instead, we should just remove --lt-dump-script and all
 related code.
 
 It was originally present because the cwrapper would emit the shwrapper,
 and then exec it.  But now, the cwrapper exec's the real program
 directly, so this functionality is no longer needed.  It hasn't been
 needed for years, but I didn't want to make too many changes at once so
 I left it there even after cwrapper was changed to direct exec the real
 program.  But...I always intended to get rid of it.  Perhaps now is the
 time.

Right, but I had already sent the first version of the line splitter when
I read this.  Here's an improved version as a proper git patch (why is it
so hard to keep your fingers away from small scriptlets?).  Now, someone
needs to decide which way to go, and provide a --lt-dump-script removal
patch if that is decided.

Hardish testing outside libtool, and limited testing with the actual
patch, but at least stresstest.at is happy with my looong PATH from the
original post, and the lt-main.c code looks good too.

Cheers,
Peter

From 9c30540472d66c0caf56bdc90a7bf7152ad771d4 Mon Sep 17 00:00:00 2001
From: Peter Rosin p...@lysator.liu.se
Date: Fri, 1 Oct 2010 22:55:55 +0200
Subject: [PATCH] cwrapper: split long lines when dumping the wrapper script.

* libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
the wrapper script contains long lines, split them for
readability and to conform with C standards.

Signed-off-by: Peter Rosin p...@lysator.liu.se
---
 ChangeLog  |7 +++
 libltdl/config/ltmain.m4sh |   11 ---
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a7aa489..515c23a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2010-10-01  Peter Rosin  p...@lysator.liu.se
+
+   cwrapper: split long lines when dumping the wrapper script.
+   * libltdl/config/ltmain.m4sh (func_emit_cwrapperexe_src): If
+   the wrapper script contains long lines, split them for
+   readability and to conform with C standards.
+
 2010-09-27  Peter Rosin  p...@lysator.liu.se
 
tests: check if sys_lib_search_path_spec works on MSVC.
diff --git a/libltdl/config/ltmain.m4sh b/libltdl/config/ltmain.m4sh
index 0418007..6eb13f0 100644
--- a/libltdl/config/ltmain.m4sh
+++ b/libltdl/config/ltmain.m4sh
@@ -4268,9 +4268,14 @@ void lt_dump_script (FILE* f)
 {
 EOF
func_emit_wrapper yes |
-  $SED -e 's/\([\\]\)/\\\1/g' \
-  -e 's/^/  fputs (/' -e 's/$/\\n, f);/'
-
+ $SED -n -e '
+s/^\(.\{79\}\)\(..*\)/\1\n\2/
+h
+s/\([\\]\)/\\\1/g
+s/$/\\n/
+s/\([^\n]*\).*/  fputs (\1, f);/p
+g
+D'
 cat EOF
 }
 EOF
-- 
1.7.1