qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] exec.c: Remove out of date comment
@ 2012-08-01 13:35 Peter Maydell
  2012-08-01 14:05 ` Avi Kivity
  2012-08-03  9:56 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
  0 siblings, 2 replies; 5+ messages in thread
From: Peter Maydell @ 2012-08-01 13:35 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-trivial, Avi Kivity, patches

Remove an out of date comment: this comment used to be attached to
cpu_register_physical_memory_log(), before commit 0f0cb164 accidentally
inserted a couple of other functions between the comment and its function.
It is in any case obsolete since (a) the function arguments it refers
to have been replaced with a single MemoryRegionSection* argument and
(b) the inability to handle regions whose offset_within_address_space
and offset_within_region aren't equally aligned was fixed as part of
the rewrite of this code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Pretty sure my analysis is right and this comment is out of date --
Avi, could you confirm that please?

 exec.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/exec.c b/exec.c
index feb4795..22dff17 100644
--- a/exec.c
+++ b/exec.c
@@ -2240,14 +2240,6 @@ static void phys_sections_clear(void)
     phys_sections_nb = 0;
 }
 
-/* register physical memory.
-   For RAM, 'size' must be a multiple of the target page size.
-   If (phys_offset & ~TARGET_PAGE_MASK) != 0, then it is an
-   io memory page.  The address used when calling the IO function is
-   the offset from the start of the region, plus region_offset.  Both
-   start_addr and region_offset are rounded down to a page boundary
-   before calculating this offset.  This should not be a problem unless
-   the low bits of start_addr and region_offset differ.  */
 static void register_subpage(MemoryRegionSection *section)
 {
     subpage_t *subpage;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] exec.c: Remove out of date comment
  2012-08-01 13:35 [Qemu-devel] [PATCH] exec.c: Remove out of date comment Peter Maydell
@ 2012-08-01 14:05 ` Avi Kivity
  2012-08-01 14:17   ` Peter Maydell
  2012-08-03  9:56 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi
  1 sibling, 1 reply; 5+ messages in thread
From: Avi Kivity @ 2012-08-01 14:05 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-trivial, qemu-devel, patches

On 08/01/2012 04:35 PM, Peter Maydell wrote:
> Remove an out of date comment: this comment used to be attached to
> cpu_register_physical_memory_log(), before commit 0f0cb164 accidentally
> inserted a couple of other functions between the comment and its function.
> It is in any case obsolete since (a) the function arguments it refers
> to have been replaced with a single MemoryRegionSection* argument and
> (b) the inability to handle regions whose offset_within_address_space
> and offset_within_region aren't equally aligned was fixed as part of
> the rewrite of this code.

(c) it doesn't use the conventional block comment style.

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Pretty sure my analysis is right and this comment is out of date --
> Avi, could you confirm that please?

Yes, you are correct.



-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] exec.c: Remove out of date comment
  2012-08-01 14:05 ` Avi Kivity
@ 2012-08-01 14:17   ` Peter Maydell
  2012-08-01 14:20     ` Avi Kivity
  0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2012-08-01 14:17 UTC (permalink / raw)
  To: Avi Kivity; +Cc: qemu-trivial, qemu-devel, patches

On 1 August 2012 15:05, Avi Kivity <avi@redhat.com> wrote:
> On 08/01/2012 04:35 PM, Peter Maydell wrote:
>> Remove an out of date comment: this comment used to be attached to
>> cpu_register_physical_memory_log(), before commit 0f0cb164 accidentally
>> inserted a couple of other functions between the comment and its function.
>> It is in any case obsolete since (a) the function arguments it refers
>> to have been replaced with a single MemoryRegionSection* argument and
>> (b) the inability to handle regions whose offset_within_address_space
>> and offset_within_region aren't equally aligned was fixed as part of
>> the rewrite of this code.
>
> (c) it doesn't use the conventional block comment style.

I agree that I don't like the aesthetics of this style of comment
but we don't actually mandate a One True Comment Format in HACKING
so I usually try to suppress my preferences in code review :-)

-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] exec.c: Remove out of date comment
  2012-08-01 14:17   ` Peter Maydell
@ 2012-08-01 14:20     ` Avi Kivity
  0 siblings, 0 replies; 5+ messages in thread
From: Avi Kivity @ 2012-08-01 14:20 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-trivial, qemu-devel, patches

On 08/01/2012 05:17 PM, Peter Maydell wrote:
> On 1 August 2012 15:05, Avi Kivity <avi@redhat.com> wrote:
>> On 08/01/2012 04:35 PM, Peter Maydell wrote:
>>> Remove an out of date comment: this comment used to be attached to
>>> cpu_register_physical_memory_log(), before commit 0f0cb164 accidentally
>>> inserted a couple of other functions between the comment and its function.
>>> It is in any case obsolete since (a) the function arguments it refers
>>> to have been replaced with a single MemoryRegionSection* argument and
>>> (b) the inability to handle regions whose offset_within_address_space
>>> and offset_within_region aren't equally aligned was fixed as part of
>>> the rewrite of this code.
>>
>> (c) it doesn't use the conventional block comment style.
> 
> I agree that I don't like the aesthetics of this style of comment
> but we don't actually mandate a One True Comment Format in HACKING
> so I usually try to suppress my preferences in code review :-)

Indeed comment style is quite low on the patch rejection scale, which is
why I didn't ask for a preliminary patch that fixes the comment style
before deleting it in the second one (giving us the option to backport
the first patch to qemu 1.1.stable if we get user complaints).


-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [Qemu-trivial] [PATCH] exec.c: Remove out of date comment
  2012-08-01 13:35 [Qemu-devel] [PATCH] exec.c: Remove out of date comment Peter Maydell
  2012-08-01 14:05 ` Avi Kivity
@ 2012-08-03  9:56 ` Stefan Hajnoczi
  1 sibling, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2012-08-03  9:56 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-trivial, patches, qemu-devel, Avi Kivity

On Wed, Aug 01, 2012 at 02:35:47PM +0100, Peter Maydell wrote:
> Remove an out of date comment: this comment used to be attached to
> cpu_register_physical_memory_log(), before commit 0f0cb164 accidentally
> inserted a couple of other functions between the comment and its function.
> It is in any case obsolete since (a) the function arguments it refers
> to have been replaced with a single MemoryRegionSection* argument and
> (b) the inability to handle regions whose offset_within_address_space
> and offset_within_region aren't equally aligned was fixed as part of
> the rewrite of this code.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Pretty sure my analysis is right and this comment is out of date --
> Avi, could you confirm that please?
> 
>  exec.c |    8 --------
>  1 file changed, 8 deletions(-)

Thanks, applied to the trivial patches tree:
https://github.com/stefanha/qemu/commits/trivial-patches

Stefan

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-08-03  9:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-01 13:35 [Qemu-devel] [PATCH] exec.c: Remove out of date comment Peter Maydell
2012-08-01 14:05 ` Avi Kivity
2012-08-01 14:17   ` Peter Maydell
2012-08-01 14:20     ` Avi Kivity
2012-08-03  9:56 ` [Qemu-devel] [Qemu-trivial] " Stefan Hajnoczi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).