[PATCH] make strace handle SIGTRAP properly

[Denys Vlasenko]

On Fri, 2011-05-27 at 02:23 +0400, Dmitry V. Levin wrote:
> On Thu, May 26, 2011 at 10:55:14AM +0200, Denys Vlasenko wrote:
> [...]
> > Please take a look at the patch below. It should have all your
> > suggestions incorporated.
> 
> Thanks, OK for me, with one more suggestion related to *error_msg*()
> implementation.
> 
> > -static void error_msg_and_die(const char *fmt, ...)
> > -#if defined __GNUC__
> > -	__attribute__ ((noreturn, format(printf, 1, 2)))
> > -#endif
> > -;
> > -static void error_msg_and_die(const char *fmt, ...)
> > +static void verror_msg(int err_and_exitflag, const char *fmt, va_list p)
> >  {
> >  	char *msg;
> > -	va_list p;
> >  
> > -	va_start(p, fmt);
> >  	msg = NULL;
> >  	vasprintf(&msg, fmt, p);
> >  	if (msg) {
> > -		fprintf(stderr, "%s: %s\n", progname, msg);
> > +		int err = err_and_exitflag & INT_MAX;
> > +		fflush(NULL);
> > +		if (err)
> > +			fprintf(stderr, "%s: %s: %s\n", progname, msg, strerror(err));
> > +		else
> > +			fprintf(stderr, "%s: %s\n", progname, msg);
> >  		free(msg);
> >  	}
> > +
> > +	if (err_and_exitflag & ((unsigned)INT_MAX+1)) {
> > +		/* Careful: don't do cleanup not from tracer process */
> > +		if (strace_tracer_pid == getpid()) {
> > +			/* TODO? cflag = 0; -- or else cleanup() may print summary */
> > +			cleanup();
> > +		}
> > +		exit(1);
> > +	}
> > +}
> 
> I think it would be more readable just to create a simple die() function
> instead:
> 
> static void die(void) __attribute__ ((noreturn))
> {
> 	if (strace_tracer_pid == getpid()) {
> 		cflag = 0;
> 		cleanup();
> 	}
> 	exit(1);
> }
> 
> > +void error_msg_and_die(const char *fmt, ...)
> > +{
> > +	va_list p;
> > +	va_start(p, fmt);
> > +	verror_msg(((unsigned)INT_MAX+1), fmt, p);
> > +	exit(1); /* verror_msg won't return. shut up compiler's warning */
> 
> verror_msg(0, fmt, p);
> die();
> 
> > +}
> > +
> > +void perror_msg(const char *fmt, ...)
> > +{
> > +	va_list p;
> > +	va_start(p, fmt);
> > +	verror_msg(errno, fmt, p);
> > +	va_end(p);
> > +}
> > +
> > +void perror_msg_and_die(const char *fmt, ...)
> > +{
> > +	va_list p;
> > +	va_start(p, fmt);
> > +	verror_msg(((unsigned)INT_MAX+1) | errno, fmt, p);
> > +	exit(1); /* verror_msg won't return. shut up compiler's warning */
> 
> verror_msg(errno, fmt, p);
> die();

Yes, this seems to be a better way to do that.
I pushed the patch to git. Thanks!

-- 
vda