From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2070 invoked by alias); 7 May 2013 20:50:04 -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 1933 invoked by uid 89); 7 May 2013 20:49:59 -0000 X-Spam-SWARE-Status: No, score=-7.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS,TW_RG autolearn=ham version=3.3.1 Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Tue, 07 May 2013 20:49:59 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r47Knqrx018695 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 7 May 2013 16:49:52 -0400 Received: from barimba (ovpn-113-163.phx2.redhat.com [10.3.113.163]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r47KnoI0015167 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 7 May 2013 16:49:50 -0400 From: Tom Tromey To: Hui Zhu Cc: Pedro Alves , Eli Zaretskii , Hui Zhu , gdb-patches ml , Marc Khouzam Subject: Re: [PATCH] add -s option to make -break-insert support dprintf References: <515451EA.1000200@mentor.com> <83y5d7wpvq.fsf@gnu.org> <516454DA.9040109@redhat.com> <87ppxzhfqy.fsf@fleche.redhat.com> <516C2549.3060808@redhat.com> <87vc7ithtj.fsf@fleche.redhat.com> <87wqrrll9m.fsf@fleche.redhat.com> Date: Tue, 07 May 2013 20:50:00 -0000 In-Reply-To: (Hui Zhu's message of "Fri, 3 May 2013 13:42:53 +0800") Message-ID: <87d2t2tt02.fsf@fleche.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-05/txt/msg00260.txt.bz2 >>>>> "Hui" == Hui Zhu writes: Hui> According to your comments. I did some update with these patches to Hui> added special command -dprintf-insert to insert dprintf. Its format Hui> is close to simple dprintf command: Hui> -dprintf-insert LOCATION FORMAT ARG ARG ... I like this approach much more. Thanks for doing it. Hui> +static char * Hui> +mi_argv_to_format (int format_num, char **argv, int argc) This needs an introductory comment explaining the arguments and result. Hui> + /* If all the string need convert to \ddd mode, so * 2. Hui> + + 2 for two ". Hui> + + 1 for \0. */ Hui> + format_size = strlen (argv[format_num]) * 4 + 3; The comment says "* 2" but the code says "* 4". Personally I'd just use an obstack rather than mess around with explicit reallocs and the like. Hui> + sprintf (format + format_current_size, "\\%o", Hui> + argv[format_num][i]); It seems that this could do the wrong thing for a char that sign-extends. Hui> + /* Apply other argv to FORMAT. */ Hui> + for (i = format_num + 1; i < argc; i++) It seems to me that it would be better to just pass in argv+argc to mi_argv_to_format, and omit the format_num argument entirely. Hui> +static void Hui> +mi_cmd_break_insert_1 (int dprintf, char *command, char **argv, int argc) Intro comment. Hui> + extra_string = mi_argv_to_format (oind + 1, argv, argc); Hui> + make_cleanup (xfree, extra_string); This makes a dangling cleanup. Hui> + if (tracepoint) Hui> + { Hui> + /* Note that to request a fast tracepoint, the client uses the Hui> + "hardware" flag, although there's nothing of hardware related to Hui> + fast tracepoints -- one can implement slow tracepoints with Hui> + hardware breakpoints, but fast tracepoints are always software. Hui> + "fast" is a misnomer, actually, "jump" would be more appropriate. Hui> + A simulator or an emulator could conceivably implement fast Hui> + regular non-jump based tracepoints. */ Hui> + type_wanted = hardware ? bp_fast_tracepoint : bp_tracepoint; Hui> + ops = &tracepoint_breakpoint_ops; Hui> + } Hui> + else if (dprintf) Hui> + { It seems that 'tracepoint' and 'dprintf' are exclusive, so this should be checked above where "hardware" is checked. Tom