The kerneldoc processor complains about the mismatched variable name.
Fix it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/memory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 6fa0b071f0..15ade918ba 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
  *
  * @notifier: the notifier to be notified
  */
-void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n);
+void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier);
 
 
 /**
-- 
2.39.2

On 10/03/2023 11.31, Alex Bennée wrote:
> The kerneldoc processor complains about the mismatched variable name.
> Fix it.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/exec/memory.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 6fa0b071f0..15ade918ba 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
>    *
>    * @notifier: the notifier to be notified
>    */
> -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n);
> +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier);

I also keep running into this problem ... I wonder whether we should run 
sphinx with "-W" to turn warnings into errors when configure has been run 
with --enable-werror ...?

Anyway, for this patch here:

Reviewed-by: Thomas Huth <thuth@redhat.com>

On Mon, 13 Mar 2023 at 17:00, Thomas Huth <thuth@redhat.com> wrote:
>
> On 10/03/2023 11.31, Alex Bennée wrote:
> > The kerneldoc processor complains about the mismatched variable name.
> > Fix it.
> >
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > ---
> >   include/exec/memory.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 6fa0b071f0..15ade918ba 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
> >    *
> >    * @notifier: the notifier to be notified
> >    */
> > -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n);
> > +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier);
>
> I also keep running into this problem ... I wonder whether we should run
> sphinx with "-W" to turn warnings into errors when configure has been run
> with --enable-werror ...?

We certainly try to do that: docs/meson.build says:

  # If we're making warnings fatal, apply this to Sphinx runs as well
  if get_option('werror')
    SPHINX_ARGS += [ '-W' ]
  endif

Has that broken ?

-- PMM

On 13/03/2023 18.03, Peter Maydell wrote:
> On Mon, 13 Mar 2023 at 17:00, Thomas Huth <thuth@redhat.com> wrote:
>>
>> On 10/03/2023 11.31, Alex Bennée wrote:
>>> The kerneldoc processor complains about the mismatched variable name.
>>> Fix it.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    include/exec/memory.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>>> index 6fa0b071f0..15ade918ba 100644
>>> --- a/include/exec/memory.h
>>> +++ b/include/exec/memory.h
>>> @@ -1738,7 +1738,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier *notifier,
>>>     *
>>>     * @notifier: the notifier to be notified
>>>     */
>>> -void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *n);
>>> +void memory_region_unmap_iommu_notifier_range(IOMMUNotifier *notifier);
>>
>> I also keep running into this problem ... I wonder whether we should run
>> sphinx with "-W" to turn warnings into errors when configure has been run
>> with --enable-werror ...?
> 
> We certainly try to do that: docs/meson.build says:
> 
>    # If we're making warnings fatal, apply this to Sphinx runs as well
>    if get_option('werror')
>      SPHINX_ARGS += [ '-W' ]
>    endif
> 
> Has that broken ?

It apparently does not work in our CI, see e.g.:

https://gitlab.com/qemu-project/qemu/-/jobs/3922732898#L1420

... there is a warning here, but the job succeeded happily.

  Thomas

On Mon, 13 Mar 2023 at 17:14, Thomas Huth <thuth@redhat.com> wrote:
>
> On 13/03/2023 18.03, Peter Maydell wrote:
> > On Mon, 13 Mar 2023 at 17:00, Thomas Huth <thuth@redhat.com> wrote:
> >> I also keep running into this problem ... I wonder whether we should run
> >> sphinx with "-W" to turn warnings into errors when configure has been run
> >> with --enable-werror ...?
> >
> > We certainly try to do that: docs/meson.build says:
> >
> >    # If we're making warnings fatal, apply this to Sphinx runs as well
> >    if get_option('werror')
> >      SPHINX_ARGS += [ '-W' ]
> >    endif
> >
> > Has that broken ?
>
> It apparently does not work in our CI, see e.g.:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/3922732898#L1420
>
> ... there is a warning here, but the job succeeded happily.

Specifically:

/builds/qemu-project/qemu/docs/../include/exec/memory.h:1741: warning:
Function parameter or member 'n' not described in
'memory_region_unmap_iommu_notifier_range'
/builds/qemu-project/qemu/docs/../include/exec/memory.h:1741: warning:
Excess function parameter 'notifier' description in
'memory_region_unmap_iommu_notifier_range'
ninja: bad depfile: multiple outputs:
/builds/qemu-project/qemu/docs/devel/secure-coding-practices.rst !=
docs/docs.stamp

Also, what's that 'bad depfile' warning from ninja about ??

I looked at the build.ninja file (which you can fish out of
the artifacts for this build), and it shows that we are
passing -W to sphinx-build:

build docs/docs.stamp: CUSTOM_COMMAND_DEP ../docs/conf.py |
/usr/bin/env /usr/bin/sphinx-build
 DEPFILE = docs/docs.d
 DEPFILE_UNQUOTED = docs/docs.d
 COMMAND = /usr/bin/env CONFDIR=etc/qemu /usr/bin/sphinx-build -q -W
-Dversion=7.2.50 -Drelease= -Ddepfile=docs/docs.d
-Ddepfile_stamp=docs/docs.stamp -b html -d
/builds/qemu-project/qemu/build/docs/manual.p
/builds/qemu-project/qemu/docs
/builds/qemu-project/qemu/build/docs/manual
 description = Generating$ docs/QEMU$ manual$ with$ a$ custom$ command

So I think the problem here is not with Sphinx, but with the
kernel-doc script. That script has an option "-Werror" which
turns its warnings into errors, but our Sphinx extension
docs/sphinx/kerneldoc.py does not set it. I think we need to
have the extension say "if Sphinx was run with -W then
pass this flag along" (hopefully Sphinx lets us find out...)

thanks
-- PMM

On Mon, 13 Mar 2023 at 17:30, Peter Maydell <peter.maydell@linaro.org> wrote:
> So I think the problem here is not with Sphinx, but with the
> kernel-doc script. That script has an option "-Werror" which
> turns its warnings into errors, but our Sphinx extension
> docs/sphinx/kerneldoc.py does not set it. I think we need to
> have the extension say "if Sphinx was run with -W then
> pass this flag along" (hopefully Sphinx lets us find out...)

This works:

--- a/docs/sphinx/kerneldoc.py
+++ b/docs/sphinx/kerneldoc.py
@@ -74,6 +74,10 @@ def run(self):
         # Sphinx versions
         cmd += ['-sphinx-version', sphinx.__version__]

+        # Pass through the warnings-as-errors flag if appropriate
+        if env.app.warningiserror:
+            cmd += ['-Werror']
+
         filename = env.config.kerneldoc_srctree + '/' + self.arguments[0]
         export_file_patterns = []


but I think it's prodding undocumented Sphinx internals, so
I'm going to check whether there's a better way to do this.
It might be more robust to have meson create a commandline
with a -Dkerneldoc_werror option that we then pick up in
the extension code, rather than trying to find out whether
-W was passed.

-- PMM

On 10/3/23 11:31, Alex Bennée wrote:
> The kerneldoc processor complains about the mismatched variable name.
> Fix it.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/exec/memory.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>