Re: cwrapper generates long strings.

[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 <address@hidden>
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 <address@hidden>
---
 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  <address@hidden>
+
+       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  <address@hidden>
 
        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