Re: [PATCH] docs/devel/loads-stores: Fix git grep regexes

[Eric Blake]

On Mon, Sep 04, 2023 at 05:17:03PM +0100, Peter Maydell wrote:
> The loads-and-stores documentation includes git grep regexes to find
> occurrences of the various functions.  Some of these regexes have
> errors, typically failing to escape the '?', '(' and ')' when they
> should be metacharacters (since these are POSIX basic REs). We also
> weren't consistent about whether to have a ':' on the end of the
> line introducing the list of regexes in each section.
> 
> Fix the errors.
> 
> The following shell rune will complain about any REs in the
> file which don't have any matches in the codebase:
>  for re in $(sed -ne 's/ - ``\(\\<.*\)``/\1/p' docs/devel/loads-stores.rst); 
> do git grep -q "$re" || echo "no matches for re $re"; done
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  docs/devel/loads-stores.rst | 40 ++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/docs/devel/loads-stores.rst b/docs/devel/loads-stores.rst
> index dab6dfa0acc..ec627aa9c06 100644
> --- a/docs/devel/loads-stores.rst
> +++ b/docs/devel/loads-stores.rst
> @@ -63,12 +63,12 @@ which stores ``val`` to ``ptr`` as an ``{endian}`` order 
> value
>  of size ``sz`` bytes.
>  
>  
> -Regexes for git grep
> +Regexes for git grep:
>   - ``\<ld[us]\?[bwlq]\(_[hbl]e\)\?_p\>``

This claims that ldul_be_p() is a valid function name (which I would
expect to take a pointer to a 32-bit integer and produce an unsigned
result suitable for assigning into a 64-bit value).  But it does not
exist, and the fact that ldl_be_p() returns 'int' means I had to add a
cast to avoid unintended sign-extension:

https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg05234.html

cast added in relation to v5 patch at
https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg04923.html

>   - ``\<st[bwlq]\(_[hbl]e\)\?_p\>``
>   - ``\<st24\(_[hbl]e\)\?_p\>``
> - - ``\<ldn_\([hbl]e\)?_p\>``
> - - ``\<stn_\([hbl]e\)?_p\>``
> + - ``\<ldn_\([hbl]e\)\?_p\>``
> + - ``\<stn_\([hbl]e\)\?_p\>``

So as long as we are touching the docs, is it worth considering the
larger task of auditing whether it is appropriate to have all of the
ld*_ functions return unsigned values, and/or implement ldu/lds
variants that guarantee zero or sign extension for widening 32-bit
values when assigning to 64-bit destinations?

>  
>  ``cpu_{ld,st}*_mmu``
>  ~~~~~~~~~~~~~~~~~~~~
> @@ -121,8 +121,8 @@ store: ``cpu_st{size}{end}_mmu(env, ptr, val, oi, 
> retaddr)``
>   - ``_le`` : little endian
>  
>  Regexes for git grep:
> - - ``\<cpu_ld[bwlq](_[bl]e)\?_mmu\>``

As a counterpoint, the cpu_ldl_* functions already return uint32_t.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org