From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=46170 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q8u9T-00046N-4P for qemu-devel@nongnu.org; Sun, 10 Apr 2011 08:54:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q8u9R-0005N9-CT for qemu-devel@nongnu.org; Sun, 10 Apr 2011 08:54:02 -0400 Received: from hall.aurel32.net ([88.191.126.93]:39646) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q8u9R-0005Mz-7k for qemu-devel@nongnu.org; Sun, 10 Apr 2011 08:54:01 -0400 Date: Sun, 10 Apr 2011 14:53:57 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH 1/3] cpu-common: Modify cpu_physical_memory_read and cpu_physical_memory_write Message-ID: <20110410125357.GA6980@volta.aurel32.net> References: <1301170017-12368-1-git-send-email-weil@mail.berlios.de> <20110409223704.GD11487@volta.aurel32.net> <4DA14B25.5000202@mail.berlios.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <4DA14B25.5000202@mail.berlios.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: Blue Swirl , QEMU Developers On Sun, Apr 10, 2011 at 08:16:05AM +0200, Stefan Weil wrote: > Am 10.04.2011 00:37, schrieb Aurelien Jarno: > >On Sat, Mar 26, 2011 at 09:06:55PM +0100, Stefan Weil wrote: > >>A lot of calls don't operate on bytes but on words or on structured data. > >>So instead of a pointer to uint8_t, a void pointer is the better choice. > >> > >>This allows removing many type casts. > >> > >>(Some very early implementations of memcpy used char pointers > >>which were replaced by void pointers for the same reason). > >> > >>Cc: Blue Swirl > >>Signed-off-by: Stefan Weil > >>--- > >>cpu-common.h | 4 ++-- > >>1 files changed, 2 insertions(+), 2 deletions(-) > >> > >>diff --git a/cpu-common.h b/cpu-common.h > >>index ef4e8da..f44a2b0 100644 > >>--- a/cpu-common.h > >>+++ b/cpu-common.h > >>@@ -68,12 +68,12 @@ void cpu_unregister_io_memory(int table_address); > >>void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf, > >>int len, int is_write); > >>static inline void cpu_physical_memory_read(target_phys_addr_t addr, > >>- uint8_t *buf, int len) > >>+ void *buf, int len) > >>{ > >>cpu_physical_memory_rw(addr, buf, len, 0); > >>} > >>static inline void cpu_physical_memory_write(target_phys_addr_t addr, > >>- const uint8_t *buf, int len) > >>+ const void *buf, int len) > >>{ > >>cpu_physical_memory_rw(addr, (uint8_t *)buf, len, 1); > > > >We might want to also do the change here, that is replacing (uint8_t *) > >to (void *). Also instead of doing half the job, it would be nice to do > >the same changes on cpu_physical_memory_rw(). > > Hello Aurelien, > > this type cast removes the const attribute from buf, so it is needed. Yes, but it can be changed to (void *) as I suggested. After your changes, what we want is to remove the const part, not change it into an uint8_t *, so I think the change improves readability. > And I did not change cpu_physical_memory_rw (and some more > functions) because the gain is small: there are only 10 type casts > used with cpu_physical_memory_rw, and at least some of them > are needed because of const attributes. > > I don't like type casts which remove the const attribute, so there > should be additional changes. But I don't think that it would be a > good idea to mix them with this patch. > Ok, that part can go into another patch. Anyway most of the use cases seems to be an abuse of cpu_physical_memory_rw(). The read or write version should be used instead of passing a constant as the last argument. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net