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 <[email protected]>
> > > > 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];



------------------------------------------------------------------------------
Special Offer -- Download ArcSight Logger for FREE!
Finally, a world-class log management solution at an even better 
price-free! And you'll get a free "Love Thy Logs" t-shirt when you
download Logger. Secure your free ArcSight Logger TODAY!
http://p.sf.net/sfu/arcsisghtdev2dev
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to