Re: [RFA] Process record and replay, 3/10

[teawater <teawater@gmail.com>]

Thanks Eli.

On Fri, Nov 14, 2008 at 19:58, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Tue, 11 Nov 2008 10:59:21 +0800
>> From: teawater <teawater@gmail.com>
>> 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)?"

I will use it.

>
>> >> +               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.

OK. When P record want make inferior stop. It will use this code.

>
>> >> +  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"?

What about "Process record target record execute log failed"?

>
>> >> +           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

OK. I will use it.

>
> By the way, is the address really a string (%s) here?

Yes, cause there is paddr_nz (addr).

>
>> >> +       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.

OK. I will use it.

>
>> >> +             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".

OK. I will use it.

>
>> >> +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.

I see. I will do it.

>
>> >> +  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\")
>

OK. I will use it.


Hui