On 02/24/16 16:52, Aaron Conole wrote:
The previous gcov behavior was to always output errors on the stderr channel.
This is fine for most uses, but some programs will require stderr to be
untouched by libgcov for certain tests. This change allows configuring
the gcov output via an environment variable which will be used to open
the appropriate file.
this is ok in principle. I have a couple of questions & nits below though.
I don't see a previous commit from you -- do you have a copyright assignment
with the FSF? (although this patch is simple, my guess is the idea it implements
is sufficiently novel to need one). We can handle that off list.
diff --git a/libgcc/libgcov-driver-system.c b/libgcc/libgcov-driver-system.c
index 4e3b244..0eb9755 100644
--- a/libgcc/libgcov-driver-system.c
+++ b/libgcc/libgcov-driver-system.c
@@ -23,6 +23,24 @@ a copy of the GCC Runtime Library Exception along with this
program;
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
+FILE *__gcov_error_file = NULL;
Unless I'm missing something, isn't this only accessed from this file? (So could
be static with a non-underbarred name)
@@ -30,12 +48,27 @@ gcov_error (const char *fmt, ...)
{
int ret;
va_list argp;
+
+ if (!__gcov_error_file)
+ __gcov_error_file = get_gcov_error_file();
Needs space before ()
+
va_start (argp, fmt);
- ret = vfprintf (stderr, fmt, argp);
+ ret = vfprintf (__gcov_error_file, fmt, argp);
va_end (argp);
return ret;
}
+#if !IN_GCOV_TOOL
And this protection here, makes me wonder what happens if one is IN_GCOV_TOOL.
Does it pay attention to GCOV_ERROR_FILE? That would seem incorrect, and thus
the above should be changed so that stderr is unconditionally used when
IN_GCOV_TOOL?
+static void
+gcov_error_exit(void)
+{
+ if (__gcov_error_file && __gcov_error_file != stderr)
+ {
Braces are not needed here.
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -46,6 +46,10 @@ void __gcov_init (struct gcov_info *p __attribute__
((unused))) {}
+ gcov_error_exit();
Needs space before ().
nathan
