Re: [PATCH v7 11/16] trace: add 'file:line' to all trace output

2014-07-02 Thread Karsten Blees
Am 02.07.2014 20:57, schrieb Junio C Hamano:
> Karsten Blees  writes:
> 
>> +#else
>> +
>> +/*
>> + * Macros to add file:line - see above for C-style declarations of how these
>> + * should be used.
>> + *
>> + * TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The
>> + * default is __FILE__, as it is consistent with assert(), and static 
>> function
>> + * names are not necessarily unique.
>> + */
>> +#define TRACE_CONTEXT __FILE__
> 
> Hmph, seeing "may be set to" forces me to wonder how.  Perhaps #ifndef/#endif
> around it?
> 

Right, shame on me. I didn't think it would be important enough to warrant a
Makefile option, but #ifndef sure wouldn't hurt.

> Also, can it be set to something like __FILE__ ":" __FUNCTION__
> which may alleviate the alleged problem of "not necessarily unique"
> perhaps?
> 

Should work with MSVC. With GCC, however, __FUNCTION__ is a string constant
supplied by the compiler, so string literal concatenation doesn't work.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 11/16] trace: add 'file:line' to all trace output

2014-07-02 Thread Junio C Hamano
Karsten Blees  writes:

>> Also, can it be set to something like __FILE__ ":" __FUNCTION__
>> which may alleviate the alleged problem of "not necessarily unique"
>> perhaps?
>
> Should work with MSVC. With GCC, however, __FUNCTION__ is a string constant
> supplied by the compiler, so string literal concatenation doesn't work.

Shucks, but not too big a deal, I guess ;-)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 11/16] trace: add 'file:line' to all trace output

2014-07-02 Thread Junio C Hamano
Karsten Blees  writes:

> +#else
> +
> +/*
> + * Macros to add file:line - see above for C-style declarations of how these
> + * should be used.
> + *
> + * TRACE_CONTEXT may be set to __FUNCTION__ if the compiler supports it. The
> + * default is __FILE__, as it is consistent with assert(), and static 
> function
> + * names are not necessarily unique.
> + */
> +#define TRACE_CONTEXT __FILE__

Hmph, seeing "may be set to" forces me to wonder how.  Perhaps #ifndef/#endif
around it?

Also, can it be set to something like __FILE__ ":" __FUNCTION__
which may alleviate the alleged problem of "not necessarily unique"
perhaps?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 11/16] trace: add 'file:line' to all trace output

2014-07-01 Thread Karsten Blees
This is useful to see where trace output came from.

Add 'const char *file, int line' parameters to the printing functions and
rename them to *_fl.

Add trace_printf* and trace_strbuf macros resolving to the *_fl functions
and let the preprocessor fill in __FILE__ and __LINE__.

As the trace_printf* functions take a variable number of arguments, this
requires variadic macros (i.e. '#define foo(...) foo_impl(__VA_ARGS__)'.
Though part of C99, it is unclear whether older compilers support this.
Thus keep the old functions and only enable variadic macros for GNUC and
MSVC 2005+ (_MSC_VER 1400). This has the nice side effect that the old
C-style declarations serve as documentation how the macros are to be used.

Print 'file:line ' as prefix to each trace line. Align the remaining trace
output at column 40 to accommodate 18 char file names + 4 digit line
number (currently there are 30 *.c files of length 18 and just 11 of 19).
Trace output from longer source files (e.g. builtin/receive-pack.c) will
not be aligned.

Signed-off-by: Karsten Blees 
---
 git-compat-util.h |  4 
 trace.c   | 72 +--
 trace.h   | 54 +
 3 files changed, 118 insertions(+), 12 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b3..4dd065d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -704,6 +704,10 @@ void git_qsort(void *base, size_t nmemb, size_t size,
 #endif
 #endif
 
+#if defined(__GNUC__) || (_MSC_VER >= 1400)
+#define HAVE_VARIADIC_MACROS 1
+#endif
+
 /*
  * Preserves errno, prints a message, but gives no warning for ENOENT.
  * Always returns the return value of unlink(2).
diff --git a/trace.c b/trace.c
index e8ce619..f013958 100644
--- a/trace.c
+++ b/trace.c
@@ -85,7 +85,8 @@ void trace_disable(struct trace_key *key)
 static const char err_msg[] = "Could not trace into fd given by "
"GIT_TRACE environment variable";
 
-static int prepare_trace_line(struct trace_key *key, struct strbuf *buf)
+static int prepare_trace_line(const char *file, int line,
+ struct trace_key *key, struct strbuf *buf)
 {
static struct trace_key trace_bare = TRACE_KEY_INIT(BARE);
struct timeval tv;
@@ -108,6 +109,14 @@ static int prepare_trace_line(struct trace_key *key, 
struct strbuf *buf)
strbuf_addf(buf, "%02d:%02d:%02d.%06ld ", tm.tm_hour, tm.tm_min,
tm.tm_sec, (long) tv.tv_usec);
 
+#ifdef HAVE_VARIADIC_MACROS
+   /* print file:line */
+   strbuf_addf(buf, "%s:%d ", file, line);
+   /* align trace output (column 40 catches most files names in git) */
+   while (buf->len < 40)
+   strbuf_addch(buf, ' ');
+#endif
+
return 1;
 }
 
@@ -121,49 +130,52 @@ static void print_trace_line(struct trace_key *key, 
struct strbuf *buf)
strbuf_release(buf);
 }
 
-static void trace_vprintf(struct trace_key *key, const char *format, va_list 
ap)
+static void trace_vprintf_fl(const char *file, int line, struct trace_key *key,
+const char *format, va_list ap)
 {
struct strbuf buf = STRBUF_INIT;
 
-   if (!prepare_trace_line(key, &buf))
+   if (!prepare_trace_line(file, line, key, &buf))
return;
 
strbuf_vaddf(&buf, format, ap);
print_trace_line(key, &buf);
 }
 
-void trace_argv_printf(const char **argv, const char *format, ...)
+static void trace_argv_vprintf_fl(const char *file, int line,
+ const char **argv, const char *format,
+ va_list ap)
 {
struct strbuf buf = STRBUF_INIT;
-   va_list ap;
 
-   if (!prepare_trace_line(NULL, &buf))
+   if (!prepare_trace_line(file, line, NULL, &buf))
return;
 
-   va_start(ap, format);
strbuf_vaddf(&buf, format, ap);
-   va_end(ap);
 
sq_quote_argv(&buf, argv, 0);
print_trace_line(NULL, &buf);
 }
 
-void trace_strbuf(struct trace_key *key, const struct strbuf *data)
+void trace_strbuf_fl(const char *file, int line, struct trace_key *key,
+const struct strbuf *data)
 {
struct strbuf buf = STRBUF_INIT;
 
-   if (!prepare_trace_line(key, &buf))
+   if (!prepare_trace_line(file, line, key, &buf))
return;
 
strbuf_addbuf(&buf, data);
print_trace_line(key, &buf);
 }
 
+#ifndef HAVE_VARIADIC_MACROS
+
 void trace_printf(const char *format, ...)
 {
va_list ap;
va_start(ap, format);
-   trace_vprintf(NULL, format, ap);
+   trace_vprintf_fl(NULL, 0, NULL, format, ap);
va_end(ap);
 }
 
@@ -171,10 +183,46 @@ void trace_printf_key(struct trace_key *key, const char 
*format, ...)
 {
va_list ap;
va_start(ap, format);
-   trace_vprintf(key, format, ap);
+   trace_vprintf_fl(NULL, 0, key, format, ap);
+   va_end(ap);
+}
+
+void trace_argv_printf(const