From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29737 invoked by alias); 17 Oct 2009 04:04:04 -0000 Received: (qmail 29729 invoked by uid 22791); 17 Oct 2009 04:04:03 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 17 Oct 2009 04:03:59 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 32E272BAC55; Sat, 17 Oct 2009 00:03:57 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id E1xngGZe4qXL; Sat, 17 Oct 2009 00:03:57 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 6E1472BAC32; Sat, 17 Oct 2009 00:03:56 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id C000FF591F; Fri, 16 Oct 2009 21:03:52 -0700 (PDT) Date: Sat, 17 Oct 2009 04:04:00 -0000 From: Joel Brobecker To: Michael Snyder Cc: "gdb-patches@sourceware.org" , Hui Zhu Subject: Re: [RFA, 1 of 3] save/restore process record, part 1 (exec_entry) Message-ID: <20091017040352.GI5272@adacore.com> References: <4AD91C32.2090900@vmware.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4AD91C32.2090900@vmware.com> User-Agent: Mutt/1.5.18 (2008-05-17) 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: 2009-10/txt/msg00381.txt.bz2 > 2009-10-16 Hui Zhu > Michael Snyder > > * record.c (record_exec_entry): New function. Emulate one > instruction, forward or backward. Abstracted from record_wait. > (record_wait) Call record_exec_entry. I can personnally only comment on details, since I don't know much about process record. Not sure who from the Global Maintainers actually know much about it except you, Michael :). > +static inline void > +record_exec_entry (struct regcache *regcache, struct gdbarch *gdbarch, > + struct record_entry *entry) We're really pushing for having all functions properly documented. Can you add a comment explaining that this function does? The function name makes it more or less obvious, I guess, but I'd personally rather have a consistent (mindless) approach of documenting everything rather than having to judge on a case-by-case basis. Also, I can't help but wonder about the use of "inline" in this case. I'm always reluctant to use this sort of feature until I can be proven that this helps performance. Since this is a static function, I would imagine that the compiler would have more knowledge of whether the function should be inlined or not? Or is that too naive? Other than than, seems like a pretty mechanical change... -- Joel