Re: [Qemu-devel] [PATCH v2] doc/memory.txt: fix typo

[Peter Maydell]

On 26 February 2016 at 10:59, Cao jin <address@hidden> wrote:
> and also modify its description, make the text parallel to the existing
> .impl.unaligned doc.

For commit messages I think they are nicer to read if they
follow the format of:

area or file: summary of changes

Longer paragraph which describes the changes in more detail.


Your commit message has a single sentence which starts in
the summary line and then runs on into the body part of the
commit message, rather than having the body part be readable
on its own.

> bonus: slightly modified the diagram, make it prettier.
> Signed-off-by: Cao jin <address@hidden>
> ---
>  docs/memory.txt | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/docs/memory.txt b/docs/memory.txt
> index 8745f76..8aee3d6 100644
> --- a/docs/memory.txt
> +++ b/docs/memory.txt
> @@ -186,15 +186,15 @@ of its own subregions: D of size 0x1000 at offset 0 and 
> E of size 0x1000 at
>  offset 0x2000. As a diagram:
>
>          0      1000   2000   3000   4000   5000   6000   7000    8000
> -        |------|------|------|------|------|------|------|-------|
> +        |------|------|------|------|------|------|------|------|
>    A:    [                                                       ]
> -  C:    [CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC]
> -  B:                  [                          ]
> -  D:                  [DDDDD]
> -  E:                                [EEEEE]
> +  C:    [CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC]
> +  B:                  [                           ]
> +  D:                  [DDDDDD]
> +  E:                                [EEEEEE]
>
>  The regions that will be seen within this address range then are:
> -        [CCCCCCCCCCCC][DDDDD][CCCCC][EEEEE][CCCCC]
> +        [CCCCCCCCCCCCC[DDDDDD]CCCCCC[EEEEEE]CCCCCC]
>
>  Since B has higher priority than C, its subregions appear in the flat map
>  even where they overlap with C. In ranges where B has not mapped anything
> @@ -203,7 +203,7 @@ C's region appears.
>  If B had provided its own MMIO operations (ie it was not a pure container)
>  then these would be used for any addresses in its range not handled by
>  D or E, and the result would be:
> -        [CCCCCCCCCCCC][DDDDD][BBBBB][EEEEE][BBBBB]
> +        [CCCCCCCCCCCCC[DDDDDD]BBBBBB[EEEEEE]BBBBBB]
>
>  Priority values are local to a container, because the priorities of two
>  regions are only compared when they are both children of the same container.

Why is this patch touching all these ascii art diagrams? If you want
to change them, that's a different patch, but I don't see any need to.
In fact you seem to have lost some of the [] from your version, so your
change doesn't look like an improvement to me.

> @@ -297,8 +297,9 @@ various constraints can be supplied to control how these 
> callbacks are called:
>   - .valid.min_access_size, .valid.max_access_size define the access sizes
>     (in bytes) which the device accepts; accesses outside this range will
>     have device and bus specific behaviour (ignored, or machine check)
> - - .valid.aligned specifies that the device only accepts naturally aligned
> -   accesses.  Unaligned accesses invoke device and bus specific behaviour.
> + - .valid.unaligned specifies that the *device being modelled* supports
> +   unaligned accesses; If false, unaligned accesses will invoke the 
> appropriate
> +   bus or CPU specific behaviour.

Should be lower case 'I' after the semicolon.

>   - .impl.min_access_size, .impl.max_access_size define the access sizes
>     (in bytes) supported by the *implementation*; other access sizes will be
>     emulated using the ones available.  For example a 4-byte write will be
> --
> 2.1.0

thanks
-- PMM