[PATCH] combining -c and regular output (repost from August 2009)

[Dmitry V. Levin]

Hi,

On Sun, Mar 28, 2010 at 02:39:38PM +0100, Adrien Kunysz wrote:
> Back in August I posted this patch that implements a new -C option. I
> got some feedback and reworked the patch accordingly but I got no follow
> up after the third iteration. Can you please consider this again or tell
> me what's wrong with this? This version applies cleanly against the
> current master branch.
> 
> This combine regular output with -c. I would find that useful to be
> integrated in the master branch in order to get rid of some custom,
> unreliable scripts I (and doubtlessy others) use to analyze regular
> strace output.

The patch looked OK, but, unfortunately, only after the first glance.
There are a lot of severe problems in trace_syscall() part of it.
You can find a fixed patch at
http://strace.git.sourceforge.net/git/gitweb.cgi?p=strace/strace;a=commitdiff;h=ldv/option-C

I had to apply these changes on top of your patch:
--- a/count.c
+++ b/count.c
@@ -50,7 +50,6 @@ static struct timeval shortest = { 1000000, 0 };
 int
 count_syscall(struct tcb *tcp, struct timeval *tv)
 {
-	tcp->flags &= ~TCB_INSYSCALL;
 	if (tcp->scno < 0 || tcp->scno >= nsyscalls)
 		return 0;
 
--- a/strace.1
+++ b/strace.1
@@ -244,7 +244,9 @@ running in the kernel) independent of wall clock time.  If -c is used with
 -f or -F (below), only aggregate totals for all traced processes are kept.
 .TP
 .B \-C
-Like -c but also print regular output while processes are running.
+Like
+.B \-c
+but also print regular output while processes are running.
 .TP
 .B \-d
 Show some debugging output of
--- a/strace.c
+++ b/strace.c
@@ -727,12 +727,20 @@ main(int argc, char *argv[])
 		"a:e:o:O:p:s:S:u:E:")) != EOF) {
 		switch (c) {
 		case 'c':
+			if (cflag == CFLAG_BOTH) {
+				fprintf(stderr, "%s: -c and -C are mutually exclusive options\n",
+					progname);
+				exit(1);
+			}
 			cflag = CFLAG_ONLY_STATS;
-			dtime++;
 			break;
 		case 'C':
+			if (cflag == CFLAG_ONLY_STATS) {
+				fprintf(stderr, "%s: -c and -C are mutually exclusive options\n",
+					progname);
+				exit(1);
+			}
 			cflag = CFLAG_BOTH;
-			dtime++;
 			break;
 		case 'd':
 			debug++;
@@ -843,8 +851,7 @@ main(int argc, char *argv[])
 
 	if (followfork > 1 && cflag) {
 		fprintf(stderr,
-			"%s: (-c or -C) and -ff are mutually exclusive options"
-			"\n",
+			"%s: (-c or -C) and -ff are mutually exclusive options\n",
 			progname);
 		exit(1);
 	}
--- a/syscall.c
+++ b/syscall.c
@@ -2369,7 +2369,7 @@ trace_syscall(struct tcb *tcp)
 		long u_error;
 
 		/* Measure the exit time as early as possible to avoid errors. */
-		if (dtime)
+		if (dtime || cflag)
 			gettimeofday(&tv, NULL);
 
 		/* BTW, why we don't just memorize syscall no. on entry
@@ -2408,9 +2408,12 @@ trace_syscall(struct tcb *tcp)
 		}
 
 		if (cflag) {
-			res = count_syscall(tcp, &tv);
-			if (cflag == CFLAG_ONLY_STATS) {
-				return res;
+			struct timeval t = tv;
+			int rc = count_syscall(tcp, &t);
+			if (cflag == CFLAG_ONLY_STATS)
+			{
+				tcp->flags &= ~TCB_INSYSCALL;
+				return rc;
 			}
 		}
 
@@ -2652,8 +2655,8 @@ trace_syscall(struct tcb *tcp)
 	}
 
 	if (cflag == CFLAG_ONLY_STATS) {
-		gettimeofday(&tcp->etime, NULL);
 		tcp->flags |= TCB_INSYSCALL;
+		gettimeofday(&tcp->etime, NULL);
 		return 0;
 	}


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20100329/d44584bb/attachment.bin>