From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-59739-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 14939 invoked by alias); 7 Nov 2008 15:34:57 -0000
Received: (qmail 14861 invoked by uid 22791); 7 Nov 2008 15:34:54 -0000
X-Spam-Check-By: sourceware.org
Received: from mtaout5.012.net.il (HELO mtaout5.012.net.il) (84.95.2.13)     by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 07 Nov 2008 15:34:11 +0000
Received: from conversion-daemon.i_mtaout5.012.net.il by i_mtaout5.012.net.il (HyperSendmail v2004.12) id <0K9Y00M00YWUAG00@i_mtaout5.012.net.il> for gdb-patches@sourceware.org; Fri, 07 Nov 2008 17:35:34 +0200 (IST)
Received: from HOME-C4E4A596F7 ([77.126.241.172]) by i_mtaout5.012.net.il (HyperSendmail v2004.12) with ESMTPA id <0K9Y00DJ1ZB68F21@i_mtaout5.012.net.il>; Fri, 07 Nov 2008 17:35:31 +0200 (IST)
Date: Fri, 07 Nov 2008 15:34:00 -0000
From: Eli Zaretskii <eliz@gnu.org>
Subject: Re: [RFA] Process record and replay, 8/10
In-reply-to: <daef60380811052350v3232a91ep6241e687f2f18be@mail.gmail.com>
X-012-Sender: halo1@inter.net.il
To: teawater <teawater@gmail.com>
Cc: gdb-patches@sourceware.org
Reply-to: Eli Zaretskii <eliz@gnu.org>
Message-id: <uljvvsg85.fsf@gnu.org>
References: <daef60380811052350v3232a91ep6241e687f2f18be@mail.gmail.com>
X-IsSubscribed: yes
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
X-SW-Source: 2008-11/txt/msg00130.txt.bz2

> Date: Thu, 6 Nov 2008 15:50:44 +0800
> From: teawater <teawater@gmail.com>
> 
> This patch add code to make I386 architecture support process record and replay.

Thanks.

> +      printf_unfiltered (_("Process record: read memeory 0x%s error.\n"),
                                                    ^^^^^^^
A typo.  (There are several more like it.)

Also, I suggest to say "error 0x%s", not "0x%s error".  The latter is
confusing for the ears of an English speaker, I think.

> +      /* XXX: index == 4 is always invalid */

Why the XXX in this comment?

> +      /* arith & logic */
> +    case 0x00 ... 0x05:
> +    case 0x08 ... 0x0d:
> +    case 0x10 ... 0x15:
> +    case 0x18 ... 0x1d:
> +    case 0x20 ... 0x25:
> +    case 0x28 ... 0x2d:
> +    case 0x30 ... 0x35:
> +    case 0x38 ... 0x3d:

Is this valid ISO C?

> +	    if (record_debug)
> +	      printf_unfiltered (_
> +				 ("Process record ignores the memory change of instruction in address 0x%s because it can't get the value of the segment register.\n"),
                                                                                           ^^^^^^^^^^
"at address".

By the way, do we need debug messages to be translatable?  Other
similar places in the patches don't have them in _().

> +    case 0x9b:
> +      printf_unfiltered (_
> +			 ("Process record don't support instruction fwait.\n"));
                                          ^^^^^^^^^^^^^
"doesn't support"

By the way, what happens if the code stream includes one of these
``unsupported'' instructions?  What will the user see at replay time?