From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52795) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tim6O-0002HT-MR for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:12:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tim6F-00031e-6l for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:11:56 -0500 Received: from mail-lb0-f173.google.com ([209.85.217.173]:51182) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tim6E-00030A-VZ for qemu-devel@nongnu.org; Wed, 12 Dec 2012 08:11:47 -0500 Received: by mail-lb0-f173.google.com with SMTP id c1so596158lbg.4 for ; Wed, 12 Dec 2012 05:11:46 -0800 (PST) Message-ID: <50C88280.6030506@gmail.com> Date: Wed, 12 Dec 2012 14:11:28 +0100 From: Antoine Mathys MIME-Version: 1.0 References: <50C7B767.8020406@gmail.com> <50C7BA7A.5060705@gmail.com> <50C87C9F.8070904@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 4/6] hw/ds1338.c: Ensure state is properly initialized. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org, paul@codesourcery.com > It's true that it probably doesn't make much difference, but > both the addr_byte and ptr are part of the migration state > listed in the vmstate struct. I think it is cleaner and > more straightforward to reason about if resetting the > device returns it to an exact known state. The state may be > undefined according to the specification but that doesn't > mean that our implementation has to leave it undefined. > > I don't understand your point about encapsulation -- these > are both fields of the device state, and ds1338_reset() > is a method of the device and has every right to access > them. You are right. While it is not strictly necessary it is cleaner (and safer) to set every field to a reasonable value. In addition some guest code probably incorrectly assumes that the pointer is initially at zero so that wouldn't hurt. Would it be ok to reply with a new version of this patch only, instead of resending the whole series ?