[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [patch] coreutils/printf.c
From:       Cristian Ionescu-Idbohrn <cristian.ionescu-idbohrn () axis ! com>
Date:       2008-05-31 20:01:39
Message-ID: 0805312143000.10516 () somehost
[Download RAW message or body]

On Sat, 31 May 2008, Denys Vlasenko wrote:

> On Saturday 31 May 2008 19:10, Cristian Ionescu-Idbohrn wrote:
> >
> > Hmm...  I must be blind :( Can't see it :(  What's wrong?  Compare to this
> > (some lines above in include/applets.h):
> >
> > USE_DIRNAME(APPLET_NOFORK(dirname, dirname, _BB_DIR_USR_BIN, _BB_SUID_NEVER, dirname))
>
> I meant that if you made it NOFORK (chunk #1) then you must not exit
> (chunk #2).
> Chunk #1 is ok if you fixed chunk #2.

Got it now.  Let's see.  Is this a better patch?


Cheers,

-- 
Cristian
["ash_builtin_printf.patch" (TEXT/x-diff)]

Index: shell/ash.c
===================================================================
--- shell/ash.c	(revision 22011)
+++ shell/ash.c	(working copy)
@@ -8488,8 +8488,9 @@
  * Apart from the above, [[ expr ]] should work as [ expr ]
  */
 
-#define testcmd test_main
-#define echocmd echo_main
+#define echocmd   echo_main
+#define printfcmd printf_main
+#define testcmd   test_main
 
 /* Keep these in proper order since it is searched via bsearch() */
 static const struct builtincmd builtintab[] = {
@@ -8540,6 +8541,9 @@
 	{ BUILTIN_NOSPEC        "let", letcmd },
 #endif
 	{ BUILTIN_ASSIGN        "local", localcmd },
+#if ENABLE_ASH_BUILTIN_PRINTF
+	{ BUILTIN_REGULAR		"printf", printfcmd },
+#endif
 	{ BUILTIN_NOSPEC        "pwd", pwdcmd },
 	{ BUILTIN_REGULAR       "read", readcmd },
 	{ BUILTIN_SPEC_REG_ASSG "readonly", exportcmd },
Index: shell/Config.in
===================================================================
--- shell/Config.in	(revision 22001)
+++ shell/Config.in	(working copy)
@@ -114,6 +114,13 @@
 	help
 	  Enable support for echo, builtin to ash.
 
+config ASH_BUILTIN_PRINTF
+	bool "Builtin version of 'printf'"
+	default y
+	depends on ASH
+	help
+	  Enable support for printf, builtin to ash.
+
 config ASH_BUILTIN_TEST
 	bool "Builtin version of 'test'"
 	default y
Index: coreutils/printf.c
===================================================================
--- coreutils/printf.c	(revision 22154)
+++ coreutils/printf.c	(working copy)
@@ -265,7 +265,7 @@
 			break;
 		case '\\':
 			if (*++f == 'c')
-				exit(EXIT_SUCCESS);
+				return 0;
 			bb_putchar(bb_process_escape_sequence((const char **)&f));
 			f--;
 			break;
@@ -277,12 +277,22 @@
 	return argv;
 }
 
-int printf_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int printf_main(int argc ATTRIBUTE_UNUSED, char **argv)
 {
 	char *format;
 	char **argv2;
 
+	/* We must check that stdout is not closed.
+	 * The reason for this is highly non-obvious.
+	 * echo_main is used from shell. Shell must correctly handle "echo foo"
+	 * if stdout is closed. With stdio, output gets shoveled into
+	 * stdout buffer, and even fflush cannot clear it out. It seems that
+	 * even if libc receives EBADF on write attempts, it feels determined
+	 * to output data no matter what. So it will try later,
+	 * and possibly will clobber future output. Not good. */
+	if (dup2(1, 1) != 1)
+		return -1;
+
 	/* bash builtin errors out on "printf '-%s-\n' foo",
 	 * coreutils-6.9 works. Both work with "printf -- '-%s-\n' foo".
 	 * We will mimic coreutils. */
@@ -304,5 +314,5 @@
 		fprintf(stderr, "excess args ignored");
 	*/
 
-	return EXIT_SUCCESS;
+	return 0;
 }
Index: include/applets.h
===================================================================
--- include/applets.h	(revision 22084)
+++ include/applets.h	(working copy)
@@ -275,7 +275,7 @@
 USE_PKILL(APPLET_ODDNAME(pkill, pgrep, _BB_DIR_USR_BIN, _BB_SUID_NEVER, pkill))
 USE_HALT(APPLET_ODDNAME(poweroff, halt, _BB_DIR_SBIN, _BB_SUID_NEVER, poweroff))
 USE_PRINTENV(APPLET(printenv, _BB_DIR_BIN, _BB_SUID_NEVER))
-USE_PRINTF(APPLET(printf, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
+USE_PRINTF(APPLET_NOFORK(printf, printf, _BB_DIR_USR_BIN, _BB_SUID_NEVER, printf))
 USE_PS(APPLET(ps, _BB_DIR_BIN, _BB_SUID_NEVER))
 USE_PSCAN(APPLET(pscan, _BB_DIR_USR_BIN, _BB_SUID_NEVER))
 USE_PWD(APPLET_NOFORK(pwd, pwd, _BB_DIR_BIN, _BB_SUID_NEVER, pwd))
Index: include/libbb.h
===================================================================
--- include/libbb.h	(revision 22110)
+++ include/libbb.h	(working copy)
@@ -882,6 +882,7 @@
 int bb_cat(char** argv);
 /* If shell needs them, these three "exist" even if not enabled as applets */
 int echo_main(int argc, char** argv) USE_ECHO(MAIN_EXTERNALLY_VISIBLE);
+int printf_main(int argc, char **argv) USE_PRINTF(MAIN_EXTERNALLY_VISIBLE);
 int test_main(int argc, char **argv) USE_TEST(MAIN_EXTERNALLY_VISIBLE);
 int kill_main(int argc, char **argv) USE_KILL(MAIN_EXTERNALLY_VISIBLE);
 int chown_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;


_______________________________________________
busybox mailing list
busybox@busybox.net
http://busybox.net/cgi-bin/mailman/listinfo/busybox

[prev in list] [next in list] [prev in thread] [next in thread]