[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] cpu-common: Modify cpu_physical_memory_read
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] cpu-common: Modify cpu_physical_memory_read and cpu_physical_memory_write |
Date: |
Sun, 10 Apr 2011 14:53:57 +0200 |
User-agent: |
Mutt/1.5.20 (2009-06-14) |
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 <address@hidden>
> >>Signed-off-by: Stefan Weil <address@hidden>
> >>---
> >>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
address@hidden http://www.aurel32.net