Message ID | alpine.DEB.2.00.1106241756410.12963@kaball-desktop |
---|---|
State | New |
Headers | show |
On 27 June 2011 14:34, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Fri, 24 Jun 2011, Peter Maydell wrote: >> You probably want to only call qemu_ram_ptr_length() if (todo). >> (I don't know if anybody ever calls this routine with a zero input >> length, but that would handle that case too.) > > I would rather fix qemu_ram_ptr_length to handle 0 as size argument, and > then call qemu_ram_ptr_length with 0 size from cpu_physical_memory_map > (see appended patch). OK, that should work too. >> (Also 'addr1' is not a fantastic variable name :-)) > > Agreed, but it is the same as before :) > Do you have any better suggestion? Maybe raddr? I admit I am not very > imaginative with names. I think raddr is better than addr1, yes. > + if (*size == 0) > + return NULL; Incidentally, QEMU coding style has braces here. scripts/checkpatch.pl can catch this kind of minor style nit for you (although it is not infallible...) -- PMM
On Mon, 27 Jun 2011, Peter Maydell wrote: > On 27 June 2011 14:34, Stefano Stabellini > <stefano.stabellini@eu.citrix.com> wrote: > > On Fri, 24 Jun 2011, Peter Maydell wrote: > >> You probably want to only call qemu_ram_ptr_length() if (todo). > >> (I don't know if anybody ever calls this routine with a zero input > >> length, but that would handle that case too.) > > > > I would rather fix qemu_ram_ptr_length to handle 0 as size argument, and > > then call qemu_ram_ptr_length with 0 size from cpu_physical_memory_map > > (see appended patch). > > OK, that should work too. > > >> (Also 'addr1' is not a fantastic variable name :-)) > > > > Agreed, but it is the same as before :) > > Do you have any better suggestion? Maybe raddr? I admit I am not very > > imaginative with names. > > I think raddr is better than addr1, yes. OK, I'll use that instead. > > + if (*size == 0) > > + return NULL; > > Incidentally, QEMU coding style has braces here. scripts/checkpatch.pl > can catch this kind of minor style nit for you (although it is not > infallible...) Thanks for the tip, I'll have to start using that script for real: I work on three different projects and they all have very very similar code styles apart from this rule about the braces in one line statements, my brain cannot cope :/
diff --git a/exec.c b/exec.c index 7f62332..e6dbddb 100644 --- a/exec.c +++ b/exec.c @@ -3137,6 +3137,8 @@ void *qemu_safe_ram_ptr(ram_addr_t addr) * but takes a size argument */ void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size) { + if (*size == 0) + return NULL; if (xen_mapcache_enabled()) return qemu_map_cache(addr, *size, 1); else { @@ -4017,7 +4019,7 @@ void *cpu_physical_memory_map(target_phys_addr_t addr, target_phys_addr_t page; unsigned long pd; PhysPageDesc *p; - ram_addr_t addr1 = addr; + ram_addr_t addr1 = ULONG_MAX; ram_addr_t rlen; void *raddr;