From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3907 invoked by alias); 16 Jan 2014 18:43:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 3880 invoked by uid 89); 16 Jan 2014 18:43:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f182.google.com Received: from mail-ie0-f182.google.com (HELO mail-ie0-f182.google.com) (209.85.223.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 16 Jan 2014 18:43:24 +0000 Received: by mail-ie0-f182.google.com with SMTP id as1so4339962iec.27 for ; Thu, 16 Jan 2014 10:43:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=cRMp58c4n5IK1tkaDBaYC97/ShR+gGkvKXHbCK5BkvI=; b=ZWNFEqxR8d+6B0/bVDYGN7tZ048tLKHByvjycV5UeCj3ovd4fUmJTgckUqTe56s0SA wM1cnGwFsFU2rEVS3igz6YwbBtqIZvGx5B/jIjH5ONzd9ptZqKMrx6Cyz2/F6fvvMpaY yFnPAORtCK8HznMAPLvC8cBQOOhLpXqGZvh27prd3SgJFzuY54NejpK6STREIOPsxFwr CJgDXwIfccQSo7FTbRRAwhj98iLnON2Ta38NXXOoUBqnJCPoKICmQ6pzNo6zCAgEiPgY mQU5RRCX/UdB+/Q5vItUoC2/TIIHQNGI4RNdcZH1HQpHgo34LVA0DfA77A28ujHW7T+F G26Q== X-Gm-Message-State: ALoCoQkZpi+N/z3DR4bKxYSyeIW8O3119wqdWmwObp/yqdHu2gVkEWWc0GAPFt4TtXxrfKr2Xz24qu2u6TOb11WRWUgHuB/oGKVxquDFfWM4DzYo1YHMJ3wbDiej9lTX3XrmlMPlU9XGs435UnFJg38Wza9J5o2Ro+sgqnsk8ivJJi23qyH+HbZwQVX9XMVdyZgTZLn6IfBRHaNQ/wfg8jo+BkgYP5jukA== MIME-Version: 1.0 X-Received: by 10.42.61.147 with SMTP id u19mr9501145ich.36.1389897802563; Thu, 16 Jan 2014 10:43:22 -0800 (PST) Received: by 10.64.58.77 with HTTP; Thu, 16 Jan 2014 10:43:22 -0800 (PST) In-Reply-To: <52D81569.3080006@redhat.com> References: <52B1842F.5020401@redhat.com> <21205.55987.69477.892571@ruffy.mtv.corp.google.com> <52D81569.3080006@redhat.com> Date: Thu, 16 Jan 2014 18:43:00 -0000 Message-ID: Subject: Re: [PATCH 4/6] gdbserver: Delimit debugging output for readability From: Doug Evans To: Pedro Alves Cc: Yao Qi , gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00639.txt.bz2 On Thu, Jan 16, 2014 at 9:22 AM, Pedro Alves wrote: >> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c >> index 13b3ff6..3706577 100644 >> --- a/gdb/gdbserver/tracepoint.c >> +++ b/gdb/gdbserver/tracepoint.c >> @@ -61,6 +61,8 @@ >> >> */ >> >> +#ifdef IN_PROCESS_AGENT >> + >> static void trace_vdebug (const char *, ...) ATTRIBUTE_PRINTF (1, 2); >> >> static void >> @@ -81,6 +83,19 @@ trace_vdebug (const char *fmt, ...) >> trace_vdebug ((fmt), ##args); \ >> } while (0) >> >> +#else >> + >> +#define trace_debug_1(level, fmt, args...) \ >> + do { \ >> + if (level <= debug_threads) \ >> + { \ >> + debug_printf ((fmt), ##args); \ >> + debug_printf ("\n"); \ >> + } \ >> + } while (0) >> + > > Please write this in a way that preserves printing PROG > in gdbserver. When debugging gdbserver and the IPA both > tracing at the same time (fast tracepoints and regular > tracepoints), it's confusing not to have there explicitly > who printed what, because both programs print the > same messages. A counter-proposal is that no information is lost given that if PROG isn't present you know it's gdbserver, and it makes the debugging output consistent with the rest of gdbserver. Otherwise "Consistency Is Good" is going to make me want to prepend PROG to all gdbserver output, which I don't have a problem with, but thought I'd double check. > >> +#endif >> + >> #define trace_debug(FMT, args...) \ >> trace_debug_1 (1, FMT, ##args) >> >> @@ -338,7 +353,7 @@ tracepoint_look_up_symbols (void) >> if (look_up_one_symbol (symbol_list[i].name, addrp, 1) == 0) >> { >> if (debug_threads) >> - fprintf (stderr, "symbol `%s' not found\n", symbol_list[i].name); >> + debug_printf ("symbol `%s' not found\n", symbol_list[i].name); >> return; >> } >> } >> diff --git a/gdb/gdbserver/utils.c b/gdb/gdbserver/utils.c >> index eff4499..1ce5512 100644 >> --- a/gdb/gdbserver/utils.c >> +++ b/gdb/gdbserver/utils.c > > Could this new debug support code be put in a new file > instead? E.g., gdbserver/debug.c ? It's a pretty-small file, and utils.c is kind of our collective kitchen sink for such things. I have no preference, just double checking that that's what you want. For one, Consistency Is Good means I'd want to push for gdb/debug.c (if/when there is something to put in there) and I want to make sure everyone is ok with that - no reason why they wouldn't be, per se, just checking. > Otherwise this all looks good to me. Thanks. >> /* Temporary storage using circular buffer. */ >> #define NUMCELLS 10 >> #define CELLSIZE 50 >> diff --git a/gdb/gdbserver/utils.h b/gdb/gdbserver/utils.h >> index 6d3df71..6c781c0 100644 >> --- a/gdb/gdbserver/utils.h >> +++ b/gdb/gdbserver/utils.h >> @@ -19,10 +19,44 @@ >> #ifndef UTILS_H >> #define UTILS_H >> >> +/* Version 2.4 and later of GCC define a magical variable `__PRETTY_FUNCTION__' >> + which contains the name of the function currently being defined. >> + This is broken in G++ before version 2.6. >> + C9x has a similar variable called __func__, but prefer the GCC one since >> + it demangles C++ function names. */ >> +#if (GCC_VERSION >= 2004) >> +#define FUNCTION_NAME __PRETTY_FUNCTION__ >> +#else >> +#if defined __STDC_VERSION__ && __STDC_VERSION__ >= 199901L >> +#define FUNCTION_NAME __func__ >> +#endif >> +#endif > > It'd be nice if this and gdb_assert.h's version of the same were > shared. That is, e.g., put this in common/common-utils.h instead, > and make gdb_assert.h define ASSERT_FUNCTION as FUNCTION_NAME > (or eliminate ASSERT_FUNCTION entirely). Are you planning on > doing it? Done in v2.