Re: [PATCH] tar: Fix build error when CONFIG_UNCOMPRESS is not selected

2015-07-01 Thread Denys Vlasenko
Rob Landley has an opposite view: preprocessor #if's are evil.

I agree with him that they do tend to obfuscate.

On Sun, Jun 28, 2015 at 9:44 AM, Michael Tokarev m...@tls.msk.ru wrote:
 [Rehashing a thread from 3 years ago]

 28.01.2013 11:48, Denys Vlasenko wrote:
 On Monday 28 January 2013 00:17, Abdoulaye Walsimou GAYE wrote:

 diff --git a/archival/libarchive/Kbuild.src 
 b/archival/libarchive/Kbuild.src
 index 58457fc..87e1ab9 100644
 --- a/archival/libarchive/Kbuild.src
 +++ b/archival/libarchive/Kbuild.src
 -lib-$(CONFIG_TAR)   += get_header_tar.o
 +lib-$(CONFIG_TAR)   += get_header_tar.o 
 decompress_uncompress.o

 /home/walsimou/embtoolkit.git/build/packages_build-mipsel-linux-mips32/busybox-1.20.2/archival/tar.c:1065:
  undefined reference to `unpack_Z_stream'
 /home/walsimou/embtoolkit.git/build/packages_build-mipsel-linux-mips32/busybox-1.20.2/archival/tar.c:1065:
  undefined reference to `unpack_Z_stream'
 /home/walsimou/embtoolkit.git/tools-mipsel-linux-mips32/bin/mipsisa32el-unknown-linux-uclibc-ld:
  busybox_unstripped: hidden symbol `unpack_Z_stream' isn't defined
 /home/walsimou/embtoolkit.git/tools-mipsel-linux-mips32/bin/mipsisa32el-unknown-linux-uclibc-ld:
  final link failed: Bad value
 mipsisa32el-unknown-linux-uclibc-clang: error: linker command failed with 
 exit code 1 (use -v to see invocation)


 I was able to build busybox-1.20.2 with this .config
 without errors.

 Looks like your compiler did not optimize this out:

 if (opt  OPT_COMPRESS)
 USE_FOR_MMU(xformer = unpack_Z_stream;)
 USE_FOR_NOMMU(xformer_prog = uncompress;)


 even though OPT_COMPRESS is a constant zero.

 The same prob exists with clang, apparently,  Here's a bugreport
 filed against debian package of busybox: http://bugs.debian.org/789499 .
 The proposed fix is to apply the above patch only if building
 with clang :)

 Basically, I'm not sure relying on dead code elimination like this is
 a good idea.  I mean, the code is eliminated, there's no if() statement
 in the generated code, so it is okay, but it looks like clang still
 records symbols referenced in the eliminated code.  Sometimes it
 is actually a good idea to keep ref symbols, eg when you build an
 executable which can load modules, so that modules will use symbols
 in that executable.

 Thanks,

 /mjt

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH 2/2] ash: use alloca to get rid of setjmp

2015-07-01 Thread Rich Felker
On Wed, Jul 01, 2015 at 04:46:18PM +0100, Ron Yorston wrote:
 Now that the only thing protected by setjmp/longjmp is the saved string,
 we can allocate it on the stack to get rid of the jump.
 
 Based on commit bd35d8e from git://git.kernel.org/pub/scm/utils/dash/dash.git
 by Herbert Xu.

In general alloca is unsafe. It's not obvious to me what the code here
is doing, so I can't tell for sure if it's safe or not, but I think
this needs a strong justification of safety before being acceptable.

Rich
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[PATCH 1/2] ash: remove parsebackquote flag

2015-07-01 Thread Ron Yorston
Commit 503a0b8 from git://git.kernel.org/pub/scm/utils/dash/dash.git
by Herbert Xu says:

  The parsebackquote flag is only used in a test where it always has the
  value zero.  So we can remove it altogether.

The first statement is incorrect:  parsebackquote is non-zero when
backquotes (as opposed to $(...)) are used for command substitution.
It is possible for the test to be executed with parsebackquote != 0 in
that case.

The test is question checks whether quotes have been closed, raising
the error unterminated quoted string if they haven't.  There seems
to be no good reason to allow unclosed quotes within backquotes.  Bash,
hush and dash (after commit 503a0b8) all treat the following as an error:

   XX=`pwd`

whereas BusyBox ash doesn't.  It just ignores the unclosed quote and
executes pwd.

So, parsebackquote should be removed but not for the reason stated.

function old new   delta
parsebackquote 1   -  -1
readtoken1  32223182 -40
--
(add/remove: 0/1 grow/shrink: 0/1 up/down: 0/-41) Total: -41 bytes

Signed-off-by: Ron Yorston r...@pobox.com
---
 shell/ash.c  | 8 +---
 shell/ash_test/ash-misc/tickquote1.right | 1 +
 shell/ash_test/ash-misc/tickquote1.tests | 1 +
 3 files changed, 3 insertions(+), 7 deletions(-)
 create mode 100644 shell/ash_test/ash-misc/tickquote1.right
 create mode 100755 shell/ash_test/ash-misc/tickquote1.tests

diff --git a/shell/ash.c b/shell/ash.c
index 282f761..1b33fbd 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -10474,7 +10474,6 @@ struct heredoc {
 };
 
 static smallint tokpushback;   /* last token pushed back */
-static smallint parsebackquote;/* nonzero if we are inside backquotes 
*/
 static smallint quoteflag; /* set if (part of) last token was 
quoted */
 static token_id_t lasttoken;   /* last token read (integer id Txxx) */
 static struct heredoc *heredoclist;/* list of here documents to read */
@@ -11314,7 +11313,7 @@ readtoken1(int c, int syntax, char *eofmark, int 
striptabs)
if (syntax == ARISYNTAX)
raise_error_syntax(missing '))');
 #endif
-   if (syntax != BASESYNTAX  !parsebackquote  eofmark == NULL)
+   if (syntax != BASESYNTAX  eofmark == NULL)
raise_error_syntax(unterminated quoted string);
if (varnest != 0) {
startlinno = g_parsefile-linno;
@@ -11610,7 +11609,6 @@ parsesub: {
  */
 parsebackq: {
struct nodelist **nlpp;
-   smallint savepbq;
union node *n;
char *volatile str;
struct jmploc jmploc;
@@ -11621,10 +11619,8 @@ parsebackq: {
 #ifdef __GNUC__
(void) saveprompt;
 #endif
-   savepbq = parsebackquote;
if (setjmp(jmploc.loc)) {
free(str);
-   parsebackquote = 0;
exception_handler = savehandler;
longjmp(exception_handler-loc, 1);
}
@@ -11708,7 +11704,6 @@ parsebackq: {
nlpp = (*nlpp)-next;
*nlpp = stzalloc(sizeof(**nlpp));
/* (*nlpp)-next = NULL; - stzalloc did it */
-   parsebackquote = oldstyle;
 
if (oldstyle) {
saveprompt = doprompt;
@@ -11742,7 +11737,6 @@ parsebackq: {
str = NULL;
INT_ON;
}
-   parsebackquote = savepbq;
exception_handler = savehandler;
USTPUTC(CTLBACKQ, out);
if (oldstyle)
diff --git a/shell/ash_test/ash-misc/tickquote1.right 
b/shell/ash_test/ash-misc/tickquote1.right
new file mode 100644
index 000..2e661bf
--- /dev/null
+++ b/shell/ash_test/ash-misc/tickquote1.right
@@ -0,0 +1 @@
+./tickquote1.tests: line 1: syntax error: unterminated quoted string
diff --git a/shell/ash_test/ash-misc/tickquote1.tests 
b/shell/ash_test/ash-misc/tickquote1.tests
new file mode 100755
index 000..90d5bbc
--- /dev/null
+++ b/shell/ash_test/ash-misc/tickquote1.tests
@@ -0,0 +1 @@
+echo `pwd`
-- 
2.4.3

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[PATCH 2/2] ash: use alloca to get rid of setjmp

2015-07-01 Thread Ron Yorston
Now that the only thing protected by setjmp/longjmp is the saved string,
we can allocate it on the stack to get rid of the jump.

Based on commit bd35d8e from git://git.kernel.org/pub/scm/utils/dash/dash.git
by Herbert Xu.

function old new   delta
readtoken1  31823116 -66
--
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-66) Total: -66 bytes

Signed-off-by: Ron Yorston r...@pobox.com
---
 shell/ash.c | 36 ++--
 1 file changed, 2 insertions(+), 34 deletions(-)

diff --git a/shell/ash.c b/shell/ash.c
index 1b33fbd..531e9e2 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -11133,19 +11133,6 @@ readtoken1(int c, int syntax, char *eofmark, int 
striptabs)
 
IF_ASH_BASH_COMPAT(smallint bash_dollar_squote = 0;)
 
-#if __GNUC__
-   /* Avoid longjmp clobbering */
-   (void) out;
-   (void) quotef;
-   (void) dblquote;
-   (void) varnest;
-   (void) arinest;
-   (void) parenlevel;
-   (void) dqvarnest;
-   (void) oldstyle;
-   (void) prevsyntax;
-   (void) syntax;
-#endif
startlinno = g_parsefile-linno;
bqlist = NULL;
quotef = 0;
@@ -11610,30 +11597,16 @@ parsesub: {
 parsebackq: {
struct nodelist **nlpp;
union node *n;
-   char *volatile str;
-   struct jmploc jmploc;
-   struct jmploc *volatile savehandler;
+   char *str;
size_t savelen;
smallint saveprompt = 0;
 
-#ifdef __GNUC__
-   (void) saveprompt;
-#endif
-   if (setjmp(jmploc.loc)) {
-   free(str);
-   exception_handler = savehandler;
-   longjmp(exception_handler-loc, 1);
-   }
-   INT_OFF;
str = NULL;
savelen = out - (char *)stackblock();
if (savelen  0) {
-   str = ckmalloc(savelen);
+   str = alloca(savelen);
memcpy(str, stackblock(), savelen);
}
-   savehandler = exception_handler;
-   exception_handler = jmploc;
-   INT_ON;
if (oldstyle) {
/* We must read until the closing backquote, giving special
 * treatment to some slashes, and then push the string and
@@ -11732,12 +11705,7 @@ parsebackq: {
if (str) {
memcpy(out, str, savelen);
STADJUST(savelen, out);
-   INT_OFF;
-   free(str);
-   str = NULL;
-   INT_ON;
}
-   exception_handler = savehandler;
USTPUTC(CTLBACKQ, out);
if (oldstyle)
goto parsebackq_oldreturn;
-- 
2.4.3

___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] ip addr: support change and replace commands

2015-07-01 Thread Denys Vlasenko
Applied, thanks.

On Tue, Jun 23, 2015 at 6:19 PM, Michael Tokarev m...@tls.msk.ru wrote:
 23.06.2015 18:44, Bernhard Reutner-Fischer wrote:
 On June 23, 2015 3:27:54 PM GMT+02:00, Michael Tokarev m...@tls.msk.ru 
 wrote:
 Ping?

 20.05.2015 16:27, Michael Tokarev wrote:
 Signed-off-by: Michael Tokarev m...@tls.msk.ru
 ---
  networking/ip.c   |  2 +-
  networking/libiproute/ipaddress.c | 20 
  2 files changed, 13 insertions(+), 9 deletions(-)

 diff --git a/networking/ip.c b/networking/ip.c
 index 98fe621..d35345c 100644
 --- a/networking/ip.c
 +++ b/networking/ip.c
 @@ -33,7 +33,7 @@
  //usage:   { {add|del} IFADDR dev STRING | {show|flush}\n
  //usage:  [dev STRING] [to PREFIX] }
  //usage:#define ipaddr_full_usage \n\n
 -//usage:   ipaddr {add|delete} IFADDR dev STRING\n
 +//usage:   ipaddr {add|change|replace|delete} IFADDR dev STRING\n
  //usage:   ipaddr {show|flush} [dev STRING] [scope SCOPE-ID]\n
  //usage:  [to PREFIX] [label PATTERN]\n
  //usage:  IFADDR := PREFIX | ADDR peer PREFIX\n
 diff --git a/networking/libiproute/ipaddress.c
 b/networking/libiproute/ipaddress.c
 index 4072d06..85f3356 100644
 --- a/networking/libiproute/ipaddress.c
 +++ b/networking/libiproute/ipaddress.c
 @@ -598,7 +598,7 @@ static int default_scope(inet_prefix *lcl)
  }

  /* Return value becomes exitcode. It's okay to not return at all */
 -static int ipaddr_modify(int cmd, char **argv)
 +static int ipaddr_modify(int cmd, int flags, char **argv)
  {
 static const char option[] ALIGN1 =
 peer\0remote\0broadcast\0brd\0
 @@ -622,7 +622,7 @@ static int ipaddr_modify(int cmd, char **argv)
 memset(req, 0, sizeof(req));

 req.n.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg));
 -   req.n.nlmsg_flags = NLM_F_REQUEST;
 +   req.n.nlmsg_flags = NLM_F_REQUEST | flags;
 req.n.nlmsg_type = cmd;
 req.ifa.ifa_family = preferred_family;

 @@ -749,16 +749,20 @@ static int ipaddr_modify(int cmd, char **argv)
  int FAST_FUNC do_ipaddr(char **argv)
  {
 static const char commands[] ALIGN1 =
 -   add\0delete\0list\0show\0lst\0flush\0;
 +   /* 01 2  3  4 5   6   7
  8 */

 +
 add\0change\0chg\0replace\0delete\0list\0show\0lst\0flush\0;
 int cmd = 2;
 if (*argv) {
 cmd = index_in_substrings(commands, *argv);
 if (cmd  0)
 invarg(*argv, applet_name);
 argv++;
 -   if (cmd = 1)
 -   return ipaddr_modify((cmd == 0) ? RTM_NEWADDR : 
 RTM_DELADDR, argv);
 -   }
 -   /* 2 == list, 3 == show, 4 == lst */
 -   return ipaddr_list_or_flush(argv, cmd == 5);
 +   if (cmd = 4)
 +   return ipaddr_modify(cmd == 4 ? RTM_DELADDR : 
 RTM_NEWADDR,

 wrong comma?

 Nope, not wrong comma.  The first argument is the command, and the second:

 +   cmd == 0 ? 
 NLM_F_CREATE|NLM_F_EXCL :
 +   cmd == 1 || cmd == 2 ? 
 NLM_F_REPLACE :
 +   cmd == 3 ? 
 NLM_F_CREATE|NLM_F_REPLACE :
 +   0, argv);

 is the flags for the command.

 +   }
 +   return ipaddr_list_or_flush(argv, cmd == 8);

 bloat-o-meter output, please.

 $ make bloatcheck
 function old new   delta
 .rodata 67076771 +64
 do_ipaddr 90 139 +49
 static.commands   32  51 +19
 ipaddr_modify   10841085  +1
 --
 (add/remove: 0/0 grow/shrink: 4/0 up/down: 133/0) Total: 133 bytes
textdata bss dec hex filename
   4714313058360   56808dde8 busybox_old
   4725513058360   56920de58 busybox_unstripped

 Thanks,

 /mjt

 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] tar: Fix build error when CONFIG_UNCOMPRESS is not selected

2015-07-01 Thread Michael Tokarev
01.07.2015 19:55, Denys Vlasenko wrote:
 Rob Landley has an opposite view: preprocessor #if's are evil.
 
 I agree with him that they do tend to obfuscate.

So what's the solution to this?  Blame compilers and force them to
act like GCC does?

Thanks,

/mjt

 On Sun, Jun 28, 2015 at 9:44 AM, Michael Tokarev m...@tls.msk.ru wrote:
 [Rehashing a thread from 3 years ago]

 28.01.2013 11:48, Denys Vlasenko wrote:
 On Monday 28 January 2013 00:17, Abdoulaye Walsimou GAYE wrote:

 diff --git a/archival/libarchive/Kbuild.src 
 b/archival/libarchive/Kbuild.src
 index 58457fc..87e1ab9 100644
 --- a/archival/libarchive/Kbuild.src
 +++ b/archival/libarchive/Kbuild.src
 -lib-$(CONFIG_TAR)   += get_header_tar.o
 +lib-$(CONFIG_TAR)   += get_header_tar.o 
 decompress_uncompress.o

 /home/walsimou/embtoolkit.git/build/packages_build-mipsel-linux-mips32/busybox-1.20.2/archival/tar.c:1065:
  undefined reference to `unpack_Z_stream'
 /home/walsimou/embtoolkit.git/build/packages_build-mipsel-linux-mips32/busybox-1.20.2/archival/tar.c:1065:
  undefined reference to `unpack_Z_stream'
 /home/walsimou/embtoolkit.git/tools-mipsel-linux-mips32/bin/mipsisa32el-unknown-linux-uclibc-ld:
  busybox_unstripped: hidden symbol `unpack_Z_stream' isn't defined
 /home/walsimou/embtoolkit.git/tools-mipsel-linux-mips32/bin/mipsisa32el-unknown-linux-uclibc-ld:
  final link failed: Bad value
 mipsisa32el-unknown-linux-uclibc-clang: error: linker command failed with 
 exit code 1 (use -v to see invocation)


 I was able to build busybox-1.20.2 with this .config
 without errors.

 Looks like your compiler did not optimize this out:

 if (opt  OPT_COMPRESS)
 USE_FOR_MMU(xformer = unpack_Z_stream;)
 USE_FOR_NOMMU(xformer_prog = uncompress;)


 even though OPT_COMPRESS is a constant zero.

 The same prob exists with clang, apparently,  Here's a bugreport
 filed against debian package of busybox: http://bugs.debian.org/789499 .
 The proposed fix is to apply the above patch only if building
 with clang :)

 Basically, I'm not sure relying on dead code elimination like this is
 a good idea.  I mean, the code is eliminated, there's no if() statement
 in the generated code, so it is okay, but it looks like clang still
 records symbols referenced in the eliminated code.  Sometimes it
 is actually a good idea to keep ref symbols, eg when you build an
 executable which can load modules, so that modules will use symbols
 in that executable.

 Thanks,

 /mjt


___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH v2] applets: Add installation of individual binaries

2015-07-01 Thread Denys Vlasenko
Applied, thanks!

On Sun, Jun 28, 2015 at 10:55 PM, Thomas Petazzoni
thomas.petazz...@free-electrons.com wrote:
 Hello,

 There has been no feedback to the below patch. We would like to support
 this feature in Buildroot, but we don't like carrying significant
 feature patches without knowing upstream's decision.

 Would the Busybox developers be willing to merge such a feature (the
 implementation details can of course be discussed/improved).

 Thanks!

 Thomas

 On Thu, 21 May 2015 14:48:35 -0500, Clayton Shotwell wrote:
 From: Clayton Shotwell clsho...@rockwellcollins.com

 Adding support to install individual binaries if the option is
 enabled. This also installs the shared libbusybox.so.* library.

 Signed-off-by: Clayton Shotwell clsho...@rockwellcollins.com

 ---
 Changes v1 - v2:
   - Changed cp -a to cp -pPR (Suggested by Bernhard)
 ---
  Makefile.custom|  4 
  applets/install.sh | 26 --
  2 files changed, 28 insertions(+), 2 deletions(-)

 diff --git a/Makefile.custom b/Makefile.custom
 index f8a1283..891c9ce 100644
 --- a/Makefile.custom
 +++ b/Makefile.custom
 @@ -28,6 +28,10 @@ ifeq ($(CONFIG_INSTALL_SH_APPLET_SCRIPT_WRAPPER),y)
  INSTALL_OPTS:= --scriptwrapper
  endif
  endif
 +ifeq ($(CONFIG_FEATURE_INDIVIDUAL),y)
 +INSTALL_OPTS:= --binaries
 +LIBBUSYBOX_SONAME:= 0_lib/libbusybox.so.$(BB_VER)
 +endif
  install: $(srctree)/applets/install.sh busybox busybox.links
   $(Q)DO_INSTALL_LIBS=$(strip $(LIBBUSYBOX_SONAME) $(DO_INSTALL_LIBS)) 
 \
   $(SHELL) $ $(CONFIG_PREFIX) $(INSTALL_OPTS)
 diff --git a/applets/install.sh b/applets/install.sh
 index 95b4719..f6c097e 100755
 --- a/applets/install.sh
 +++ b/applets/install.sh
 @@ -5,19 +5,26 @@ export LC_CTYPE=POSIX

  prefix=$1
  if [ -z $prefix ]; then
 - echo usage: applets/install.sh DESTINATION 
 [--symlinks/--hardlinks/--scriptwrapper]
 + echo usage: applets/install.sh DESTINATION 
 [--symlinks/--hardlinks/--binaries/--scriptwrapper]
   exit 1
  fi

 +# Source the configuration
 +. ./.config
 +
  h=`sort busybox.links | uniq`

 +sharedlib_dir=0_lib
 +
  linkopts=
  scriptwrapper=n
 +binaries=n
  cleanup=0
  noclobber=0
  case $2 in
   --hardlinks) linkopts=-f;;
   --symlinks)  linkopts=-fs;;
 + --binaries)  binaries=y;;
   --scriptwrapper) scriptwrapper=y;swrapall=y;;
   --sw-sh-hard)scriptwrapper=y;linkopts=-f;;
   --sw-sh-sym) scriptwrapper=y;linkopts=-fs;;
 @@ -40,8 +47,9 @@ if [ -n $DO_INSTALL_LIBS ]  [ $DO_INSTALL_LIBS != 
 n ]; then
   for i in $DO_INSTALL_LIBS; do
   rm -f $prefix/$libdir/$i || exit 1
   if [ -f $i ]; then
 + echoInstalling $i to the target at 
 $prefix/$libdir/
   cp -pPR $i $prefix/$libdir/ || exit 1
 - chmod 0644 $prefix/$libdir/$i || exit 1
 + chmod 0644 $prefix/$libdir/`basename $i` || exit 1
   fi
   done
  fi
 @@ -68,6 +76,7 @@ install -m 755 busybox $prefix/bin/busybox || exit 1

  for i in $h; do
   appdir=`dirname $i`
 + app=`basename $i`
   mkdir -p $prefix/$appdir || exit 1
   if [ $scriptwrapper = y ]; then
   if [ $swrapall != y ]  [ $i = /bin/sh ]; then
 @@ -78,6 +87,19 @@ for i in $h; do
   chmod +x $prefix/$i
   fi
   echo   $prefix/$i
 + elif [ $binaries = y ]; then
 + # Copy the binary over rather
 + if [ -e $sharedlib_dir/$app ]; then
 + if [ $noclobber = 0 ] || [ ! -e $prefix/$i ]; 
 then
 + echoCopying $sharedlib_dir/$app to 
 $prefix/$i
 + cp -pPR $sharedlib_dir/$app $prefix/$i || exit 
 1
 + else
 + echo   $prefix/$i already exists
 + fi
 + else
 + echo Error: Could not find $sharedlib_dir/$app
 + exit 1
 + fi
   else
   if [ $2 = --hardlinks ]; then
   bb_path=$prefix/bin/busybox



 --
 Thomas Petazzoni, CTO, Free Electrons
 Embedded Linux, Kernel and Android engineering
 http://free-electrons.com
 ___
 busybox mailing list
 busybox@busybox.net
 http://lists.busybox.net/mailman/listinfo/busybox
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox