[SCM] strace branch, master, updated. v4.6-115-g102ec49

Denys Vlasenko dvlasenk at redhat.com
Wed Aug 31 13:19:16 UTC 2011


On Wed, 2011-08-31 at 16:25 +0400, Dmitry V. Levin wrote:
> On Wed, Aug 31, 2011 at 02:04:52PM +0200, Denys Vlasenko wrote:
> > On Tue, 2011-08-30 at 20:20 +0400, Dmitry V. Levin wrote:
> > > On Wed, Aug 24, 2011 at 11:41:10PM +0000, Denys Vlasenko wrote:
> > > > commit 102ec4935440ff52a7fa3566154a84cc2473f16a
> > > > Author: Denys Vlasenko <dvlasenk at redhat.com>
> > > > Date:   Thu Aug 25 01:27:59 2011 +0200
> > > > 
> > > >     Optimize tabto()
> > > >     
> > > >     tabto is used in many lines of strace output.
> > > >     On glibc, tprintf("%*s", col - curcol, "") is noticeably slow
> > > >     compared to tprintf("                 "). Use the latter.
> > > >     Observed ~15% reduction of time spent in userspace.
> > > >     
> > > >     * defs.h: Drop extern declaration of acolumn. Make tabto()
> > > >     take no parameters.
> > > >     * process.c (sys_exit): Call tabto() with no parameters.
> > > >     * syscall.c (trace_syscall_exiting): Call tabto() with no parameters.
> > > >     * strace.c: Make acolumn static, add static char *acolumn_spaces.
> > > >     (main): Allocate acolumn_spaces as a string of spaces.
> > > >     (printleader): Call tabto() with no parameters.
> > > >     (tabto): Use simpler method to print lots of spaces.
> > > [...]
> > > >  void
> > > > -tabto(int col)
> > > > +tabto(void)
> > > >  {
> > > > -	if (curcol < col)
> > > > -		tprintf("%*s", col - curcol, "");
> > > > +	if (curcol < acolumn)
> > > > +		tprintf(acolumn_spaces + curcol);
> > > >  }
> > > 
> > > The new statement yields a warning:
> > > 
> > > strace.c: In function 'tabto':
> > > strace.c:2701:3: warning: format not a string literal and no format arguments
> > 
> > Pity, my toolchain doesn't emit that... I can't detect these warnings.
> 
> My toolchain enables -Wformat-security by default.  We can get it enabled
> by default in strace by adding gl_WARN_ADD([-Wformat-security]) to
> configure.ac
> 
> > > Maybe we could use
> > > 		tprintf("%s", acolumn_spaces + curcol);
> > > instead without performance degradation?
> > 
> > There will be some performance degradation.
> > Is there a way to suppress this warning on a case-by-case basis?
> 
> I'm not aware of such a way.  If the performance degradation is
> noticeable, a straightforward function tprint_str(const char *str) could
> be added to do exactly that.  The implementation could use fputs() instead
> of vfprintf(), and that would speedup things even more than now. ;)


Something like this?

-- 
vda



diff -d -urpN strace.4/defs.h strace.5/defs.h
--- strace.4/defs.h	2011-08-31 13:53:17.000000000 +0200
+++ strace.5/defs.h	2011-08-31 15:04:13.625365787 +0200
@@ -708,6 +708,7 @@ extern int proc_open(struct tcb *tcp, in
 	printtv_bitness((tcp), (addr), BITNESS_CURRENT, 1)
 
 extern void tprintf(const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));
+extern void tprints(const char *str);
 
 #ifndef HAVE_STRERROR
 const char *strerror(int);
diff -d -urpN strace.4/desc.c strace.5/desc.c
--- strace.4/desc.c	2011-08-31 13:53:17.000000000 +0200
+++ strace.5/desc.c	2011-08-31 15:05:22.492264800 +0200
@@ -519,7 +519,7 @@ decode_select(struct tcb *tcp, long *arg
 			tprintf(", [");
 			for (j = 0, sep = ""; j < nfds; j++) {
 				if (FD_ISSET(j, fds)) {
-					tprintf("%s", sep);
+					tprints(sep);
 					printfd(tcp, j);
 					sep = " ";
 				}
diff -d -urpN strace.4/file.c strace.5/file.c
--- strace.4/file.c	2011-08-31 13:53:17.000000000 +0200
+++ strace.5/file.c	2011-08-31 15:05:30.699367714 +0200
@@ -386,7 +386,7 @@ sprint_open_modes(mode_t flags)
 void
 tprint_open_modes(mode_t flags)
 {
-	tprintf("%s", sprint_open_modes(flags) + sizeof("flags"));
+	tprints(sprint_open_modes(flags) + sizeof("flags"));
 }
 
 static int
diff -d -urpN strace.4/io.c strace.5/io.c
--- strace.4/io.c	2011-08-31 10:47:35.000000000 +0200
+++ strace.5/io.c	2011-08-31 15:05:42.332513580 +0200
@@ -426,7 +426,7 @@ sys_ioctl(struct tcb *tcp)
 		tprintf(", ");
 		iop = ioctl_lookup(tcp->u_arg[1]);
 		if (iop) {
-			tprintf("%s", iop->symbol);
+			tprints(iop->symbol);
 			while ((iop = ioctl_next_match(iop)))
 				tprintf(" or %s", iop->symbol);
 		} else
diff -d -urpN strace.4/net.c strace.5/net.c
--- strace.4/net.c	2011-08-31 10:47:35.000000000 +0200
+++ strace.5/net.c	2011-08-31 15:05:54.317663846 +0200
@@ -1468,7 +1468,7 @@ tprint_sock_type(struct tcb *tcp, int fl
 	const char *str = xlookup(socktypes, flags & SOCK_TYPE_MASK);
 
 	if (str) {
-		tprintf("%s", str);
+		tprints(str);
 		flags &= ~SOCK_TYPE_MASK;
 		if (!flags)
 			return;
diff -d -urpN strace.4/process.c strace.5/process.c
--- strace.4/process.c	2011-08-31 10:47:35.000000000 +0200
+++ strace.5/process.c	2011-08-31 15:16:51.807428788 +0200
@@ -1505,7 +1505,7 @@ printargv(struct tcb *tcp, long addr)
 			cp.p64 = cp.p32;
 		if (cp.p64 == 0)
 			break;
-		tprintf("%s", sep);
+		tprints(sep);
 		printstr(tcp, cp.p64, -1);
 		addr += personality_wordsize[current_personality];
 	}
diff -d -urpN strace.4/signal.c strace.5/signal.c
--- strace.4/signal.c	2011-08-31 10:47:35.000000000 +0200
+++ strace.5/signal.c	2011-08-31 15:07:43.118027310 +0200
@@ -385,13 +385,13 @@ sprintsigmask(const char *str, sigset_t 
 static void
 printsigmask(sigset_t *mask, int rt)
 {
-	tprintf("%s", sprintsigmask("", mask, rt));
+	tprints(sprintsigmask("", mask, rt));
 }
 
 void
 printsignal(int nr)
 {
-	tprintf("%s", signame(nr));
+	tprints(signame(nr));
 }
 
 void
diff -d -urpN strace.4/strace.c strace.5/strace.c
--- strace.4/strace.c	2011-08-31 13:59:59.000000000 +0200
+++ strace.5/strace.c	2011-08-31 15:12:57.125956341 +0200
@@ -2646,7 +2646,21 @@ tprintf(const char *fmt, ...)
 			curcol += n;
 	}
 	va_end(args);
-	return;
+}
+
+void
+tprints(const char *str)
+{
+	if (outf) {
+		int n = fputs(str, outf);
+		if (n >= 0) {
+			curcol += strlen(str);
+			return;
+		}
+		if (outf != stderr)
+			perror(outfname == NULL
+			       ? "<writing to pipe>" : outfname);
+	}
 }
 
 void
@@ -2705,7 +2719,7 @@ void
 tabto(void)
 {
 	if (curcol < acolumn)
-		tprintf(acolumn_spaces + curcol);
+		tprints(acolumn_spaces + curcol);
 }
 
 void
diff -d -urpN strace.4/stream.c strace.5/stream.c
--- strace.4/stream.c	2011-08-31 12:26:01.000000000 +0200
+++ strace.5/stream.c	2011-08-31 15:13:28.999354690 +0200
@@ -742,17 +742,17 @@ print_transport_message(struct tcb *tcp,
 #define GET(type, struct)	\
 	do {							\
 		if (len < sizeof m.struct) goto dump;		\
-		if (umove(tcp, addr, &m.struct) < 0) goto dump;\
-		tprintf("{");					\
+		if (umove(tcp, addr, &m.struct) < 0) goto dump;	\
+		tprints("{");					\
 		if (expect != type) {				\
 			++c;					\
-			tprintf(#type);			\
+			tprints(#type);				\
 		}						\
 	}							\
 	while (0)
 
 #define COMMA() \
-	do { if (c++) tprintf(", "); } while (0)
+	do { if (c++) tprints(", "); } while (0)
 
 
 #define STRUCT(struct, elem, print)					\
@@ -767,7 +767,7 @@ print_transport_message(struct tcb *tcp,
 				m.struct.elem##_offset);		\
 		}							\
 		else {							\
-			tprintf(#elem "=");				\
+			tprints(#elem "=");				\
 			print(tcp,					\
 				 addr + m.struct.elem##_offset,		\
 				 m.struct.elem##_length);		\
diff -d -urpN strace.4/syscall.c strace.5/syscall.c
--- strace.4/syscall.c	2011-08-31 13:53:17.000000000 +0200
+++ strace.5/syscall.c	2011-08-31 15:05:59.004722609 +0200
@@ -683,7 +683,7 @@ sys_indir(struct tcb *tcp)
 			return 0;
 		}
 		nargs = sysent[scno].nargs;
-		tprintf("%s", sysent[scno].sys_name);
+		tprints(sysent[scno].sys_name);
 		for (i = 0; i < nargs; i++)
 			tprintf(", %#lx", tcp->u_arg[i+1]);
 	}
diff -d -urpN strace.4/util.c strace.5/util.c
--- strace.4/util.c	2011-08-31 13:53:17.000000000 +0200
+++ strace.5/util.c	2011-08-31 15:16:51.808428798 +0200
@@ -236,7 +236,7 @@ printxval(const struct xlat *xlat, int v
 	const char *str = xlookup(xlat, val);
 
 	if (str)
-		tprintf("%s", str);
+		tprints(str);
 	else
 		tprintf("%#x /* %s */", val, dflt);
 }
@@ -340,7 +340,7 @@ printflags(const struct xlat *xlat, int 
 	const char *sep;
 
 	if (flags == 0 && xlat->val == 0) {
-		tprintf("%s", xlat->str);
+		tprints(xlat->str);
 		return 1;
 	}
 
@@ -423,8 +423,7 @@ printfd(struct tcb *tcp, int fd)
 void
 printuid(const char *text, unsigned long uid)
 {
-	tprintf("%s", text);
-	tprintf((uid == -1) ? "%ld" : "%lu", uid);
+	tprintf((uid == -1) ? "%s%ld" : "%s%lu", text, uid);
 }
 
 static char path[MAXPATHLEN + 1];






More information about the Strace-devel mailing list