From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28505 invoked by alias); 14 Nov 2008 11:58:48 -0000 Received: (qmail 28424 invoked by uid 22791); 14 Nov 2008 11:58:47 -0000 X-Spam-Check-By: sourceware.org Received: from mtaout1.012.net.il (HELO mtaout1.012.net.il) (84.95.2.1) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 14 Nov 2008 11:58:05 +0000 Received: from conversion-daemon.i-mtaout1.012.net.il by i-mtaout1.012.net.il (HyperSendmail v2007.08) id <0KAB00800NGAIP00@i-mtaout1.012.net.il> for gdb-patches@sourceware.org; Fri, 14 Nov 2008 13:59:55 +0200 (IST) Received: from HOME-C4E4A596F7 ([77.126.205.49]) by i-mtaout1.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0KAB00G4CNZS7JD0@i-mtaout1.012.net.il>; Fri, 14 Nov 2008 13:59:53 +0200 (IST) Date: Fri, 14 Nov 2008 16:27:00 -0000 From: Eli Zaretskii Subject: Re: [RFA] Process record and replay, 3/10 In-reply-to: X-012-Sender: halo1@inter.net.il To: teawater Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: References: X-IsSubscribed: yes 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 X-SW-Source: 2008-11/txt/msg00334.txt.bz2 > Date: Tue, 11 Nov 2008 10:59:21 +0800 > From: teawater > Cc: gdb-patches@sourceware.org > > >> + q = yquery (_("The record instruction number (record-insn-number) is equal to record-insn-number-max. Do you want to close record/replay stop when record/replay buffer becomes full(record-stop-at-limit) then auto delete first record_t?")); > > > > There's something wrong with this query. First, why both "close" and > > "stop"? Also, what is "record_t"? a typo? > > What about change it to "Do you want to auto delete first execute log > entry when record/replay buffer becomes full(record-stop-at-limit)?" I suggest "Do you want to auto delete previous execute log entries when record/replay buffer becomes full (record-stop-at-limit)?" > >> + error (_("Process record: record stop the program.")); > > > > Do you mean > > > > Process record: program recording stopped. > > > > ? > > What about "Process record stop inferior." I'm not sure. Can you explain what exactly does this message try to say? I will then suggest a better wording. > >> + if (ret < 0) > >> + error (_("Process record record message error.")); > > > > Do you mean something like "Process record error."? That is, does > > this happen when some error is encountered inside > > gdbarch_process_record? > > > > What about "Process record execute log error"? "Error executing process log record"? > >> + if (target_read_memory > >> + (record_list->u.mem.addr, mem, record_list->u.mem.len)) > >> + { > >> + error (_("Process record: read memory addr = 0x%s len = %d error."), > > > > Here also, the error message should be more clearly phrased. > > Could you tell me how to output this message clear? "Error reading memory at addr = 0x%s, len = %d By the way, is the address really a string (%s) here? > >> + if (record_arch_list_add_reg (i)) > >> + { > >> + record_list_release (record_arch_list_tail); > >> + error (_("Process record: record message error.")); > > > > Same here. (There are more like this one.) > > Change it to "record execute log"? Yes, that's better. > >> + nquery (_ > >> + ("Becuse GDB is in replay mode, changing the value of a register will destroy the record from this point forward. Change all register?")); > > > > "all registers", in plural. > > > > Also, I'm not sure I would understand what you mean by ``destroy the > > record''? Are you saying that process recording will effectively stop > > working from this point onward? > > What about change it to "destory the execute log"? I think you mean "make the execute log unusable from this point onward". > >> +static void > >> +set_record_insn_max_num (char *args, int from_tty, struct cmd_list_element *c) > >> +{ > >> + if (record_insn_num > record_insn_max_num && record_insn_max_num) > >> + { > >> + printf_unfiltered (_("Process record: record instructions number is bigger than record instructions max number. Auto delete the first ones.\n")); > > > > And here. Also, we need a question mark at the end of the second > > sentence. > > I think maybe we need let user know when delete some log entries. Yes, but I didn't mean to remove the question, only the "Process record:" prefix. > >> + add_com ("delrecord", class_obscure, cmd_record_delete, > >> + _("When process record target running in replay mode, delete the next running messages and begin to record the running message at current address.")); > > > > This doc string should be made shorter, to fit a single terminal line, > > and the first line should not include comma characters, because > > otherwise some help commands will not display anything beyond the > > first comma. > > > > What about this: > _("When process record target running in replay mode,\n\ > delete the next running messages and begin to record\n\ > the execute log at current address.") The first line must be a complete sentence, because some help commands (e.g., `apropos') show only the first line. I suggest _("Delete the rest of execution log and start recording it anew.\n\")