[PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

Hi Jon,

according to your remarks I fixed the first and second patch. The third patch is
resend unchanged;

> Am 06.09.2016 um 14:28 schrieb Jonathan Corbet <corbet@lwn.net>:
>
> As others have pointed out, we generally want to hide the difference
> between functions and macros, so this is probably one change we don't
> want.

I read "probably", so there might be a chance to persuade you ;)

I'm not a friend of *information hiding* and since the index is sorted
alphabetical it does no matter if the entry is 'FOO (C function)' or 'FOO (C
macro)'. The last one has the right information e.g. for someone how is looking
for a macro. FOO is a function-like macro and not a function, if the author
describes the macro he might use the word "macro FOO" but in the index it is
tagged as C function.

Macros and functions are totally different even if their notation looks
similarly. So where is the benefit of entries like 'FOO (C function)', which is
IMHO ambiguous.

I tagged the 'function-like macros index entry' patch with 'RFC' and resend it
within this series. If you and/or others have a different opinion, feel free to
drop it.

Thanks for review.

-- Markus --


Markus Heiser (3):
  doc-rst:c-domain: fix sphinx version incompatibility
  doc-rst:c-domain: function-like macros arguments
  doc-rst:c-domain: function-like macros index entry

 Documentation/sphinx/cdomain.py | 79 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH v2 1/3] doc-rst:c-domain: fix sphinx version incompatibility

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

The self.indexnode's tuple has changed in sphinx version 1.4, from a
former 4 element tuple to a 5 element tuple.

https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c

Signed-off-by: Markus Heiser <markus.heiser@darmarIT.de>
---
 Documentation/sphinx/cdomain.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index 9eb714a..5f0c7ed 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -29,11 +29,15 @@ u"""
 
 from docutils.parsers.rst import directives
 
+import sphinx
 from sphinx.domains.c import CObject as Base_CObject
 from sphinx.domains.c import CDomain as Base_CDomain
 
 __version__  = '1.0'
 
+# Get Sphinx version
+major, minor, patch = map(int, sphinx.__version__.split("."))
+
 def setup(app):
 
     app.override_domain(CDomain)
@@ -85,8 +89,14 @@ class CObject(Base_CObject):
 
         indextext = self.get_index_text(name)
         if indextext:
-            self.indexnode['entries'].append(('single', indextext,
-                                              targetname, '', None))
+            if major == 1 and minor < 4:
+                # indexnode's tuple changed in 1.4
+                # https://github.com/sphinx-doc/sphinx/commit/e6a5a3a92e938fcd75866b4227db9e0524d58f7c
+                self.indexnode['entries'].append(
+                    ('single', indextext, targetname, ''))
+            else:
+                self.indexnode['entries'].append(
+                    ('single', indextext, targetname, '', None))
 
 class CDomain(Base_CDomain):
 
-- 
2.7.4

[PATCH v2 2/3] doc-rst:c-domain: function-like macros arguments

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

Handle signatures of function-like macros well. Don't try to deduce
arguments types of function-like macros.

Signed-off-by: Markus Heiser <markus.heiser@darmarIT.de>
---
 Documentation/sphinx/cdomain.py | 55 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index 5f0c7ed..df0419c 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -1,4 +1,5 @@
 # -*- coding: utf-8; mode: python -*-
+# pylint: disable=W0141,C0113,C0103,C0325
 u"""
     cdomain
     ~~~~~~~
@@ -25,11 +26,18 @@ u"""
 
           * :c:func:`VIDIOC_LOG_STATUS` or
           * :any:`VIDIOC_LOG_STATUS` (``:any:`` needs sphinx 1.3)
+
+     * Handle signatures of function-like macros well. Don't try to deduce
+       arguments types of function-like macros.
+
 """
 
+from docutils import nodes
 from docutils.parsers.rst import directives
 
 import sphinx
+from sphinx import addnodes
+from sphinx.domains.c import c_funcptr_sig_re, c_sig_re
 from sphinx.domains.c import CObject as Base_CObject
 from sphinx.domains.c import CDomain as Base_CDomain
 
@@ -57,9 +65,54 @@ class CObject(Base_CObject):
         "name" : directives.unchanged
     }
 
+    def handle_func_like_macro(self, sig, signode):
+        u"""Handles signatures of function-like macros.
+
+        If the objtype is 'function' and the the signature ``sig`` is a
+        function-like macro, the name of the macro is returned. Otherwise
+        ``False`` is returned.  """
+
+        if not self.objtype == 'function':
+            return False
+
+        m = c_funcptr_sig_re.match(sig)
+        if m is None:
+            m = c_sig_re.match(sig)
+            if m is None:
+                raise ValueError('no match')
+
+        rettype, fullname, arglist, _const = m.groups()
+        arglist = arglist.strip()
+        if rettype or not arglist:
+            return False
+
+        arglist = arglist.replace('`', '').replace('\\ ', '') # remove markup
+        arglist = [a.strip() for a in arglist.split(",")]
+
+        # has the first argument a type?
+        if len(arglist[0].split(" ")) > 1:
+            return False
+
+        # This is a function-like macro, it's arguments are typeless!
+        signode  += addnodes.desc_name(fullname, fullname)
+        paramlist = addnodes.desc_parameterlist()
+        signode  += paramlist
+
+        for argname in arglist:
+            param = addnodes.desc_parameter('', '', noemph=True)
+            # separate by non-breaking space in the output
+            param += nodes.emphasis(argname, argname)
+            paramlist += param
+
+        return fullname
+
     def handle_signature(self, sig, signode):
         """Transform a C signature into RST nodes."""
-        fullname = super(CObject, self).handle_signature(sig, signode)
+
+        fullname = self.handle_func_like_macro(sig, signode)
+        if not fullname:
+            fullname = super(CObject, self).handle_signature(sig, signode)
+
         if "name" in self.options:
             if self.objtype == 'function':
                 fullname = self.options["name"]
-- 
2.7.4

[RFC v2 3/3] doc-rst:c-domain: function-like macros index entry

[Markus Heiser]

From: Markus Heiser <markus.heiser@darmarIT.de>

For function-like macros, sphinx creates 'FOO (C function)' entries.
With this patch 'FOO (C macro)' are created for function-like macros,
which is the same for object-like macros.

Signed-off-by: Markus Heiser <markus.heiser@darmarIT.de>
---
 Documentation/sphinx/cdomain.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/sphinx/cdomain.py b/Documentation/sphinx/cdomain.py
index df0419c..ead81b5 100644
--- a/Documentation/sphinx/cdomain.py
+++ b/Documentation/sphinx/cdomain.py
@@ -37,6 +37,7 @@ from docutils.parsers.rst import directives
 
 import sphinx
 from sphinx import addnodes
+from sphinx.locale import _
 from sphinx.domains.c import c_funcptr_sig_re, c_sig_re
 from sphinx.domains.c import CObject as Base_CObject
 from sphinx.domains.c import CDomain as Base_CDomain
@@ -65,6 +66,8 @@ class CObject(Base_CObject):
         "name" : directives.unchanged
     }
 
+    is_function_like_macro = False
+
     def handle_func_like_macro(self, sig, signode):
         u"""Handles signatures of function-like macros.
 
@@ -104,6 +107,7 @@ class CObject(Base_CObject):
             param += nodes.emphasis(argname, argname)
             paramlist += param
 
+        self.is_function_like_macro = True
         return fullname
 
     def handle_signature(self, sig, signode):
@@ -151,6 +155,12 @@ class CObject(Base_CObject):
                 self.indexnode['entries'].append(
                     ('single', indextext, targetname, '', None))
 
+    def get_index_text(self, name):
+        if self.is_function_like_macro:
+            return _('%s (C macro)') % name
+        else:
+            return super(CObject, self).get_index_text(name)
+
 class CDomain(Base_CDomain):
 
     """C language domain."""
-- 
2.7.4

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Mauro Carvalho Chehab]

Em Wed,  7 Sep 2016 09:12:55 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> From: Markus Heiser <markus.heiser@darmarIT.de>
> 
> Hi Jon,
> 
> according to your remarks I fixed the first and second patch. The third patch is
> resend unchanged;
> 
> > Am 06.09.2016 um 14:28 schrieb Jonathan Corbet <corbet@lwn.net>:
> >
> > As others have pointed out, we generally want to hide the difference
> > between functions and macros, so this is probably one change we don't
> > want.  
> 
> I read "probably", so there might be a chance to persuade you ;)
> 
> I'm not a friend of *information hiding* and since the index is sorted
> alphabetical it does no matter if the entry is 'FOO (C function)' or 'FOO (C
> macro)'. The last one has the right information e.g. for someone how is looking
> for a macro. FOO is a function-like macro and not a function, if the author
> describes the macro he might use the word "macro FOO" but in the index it is
> tagged as C function.
> 
> Macros and functions are totally different even if their notation looks
> similarly. So where is the benefit of entries like 'FOO (C function)', which is
> IMHO ambiguous.
> 
> I tagged the 'function-like macros index entry' patch with 'RFC' and resend it
> within this series. If you and/or others have a different opinion, feel free to
> drop it.
> 
> Thanks for review.
> 
> -- Markus --
> 
> 
> Markus Heiser (3):
>   doc-rst:c-domain: fix sphinx version incompatibility
>   doc-rst:c-domain: function-like macros arguments
>   doc-rst:c-domain: function-like macros index entry
> 
>  Documentation/sphinx/cdomain.py | 79 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 76 insertions(+), 3 deletions(-)
> 

Those patches indeed fix the issues. The arguments are now
processed properly.

Tested-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>

---

Using either this approach or my kernel-doc patch, I'm now getting
only two warnings:

1) at media-entity.h, even without nitpick mode:

./include/media/media-entity.h:1053: warning: No description found for parameter '...'

This is caused by this kernel-doc tag and the corresponding macro:

	/**
	 * media_entity_call - Calls a struct media_entity_operations operation on
	 *	an entity
	 *
	 * @entity: entity where the @operation will be called
	 * @operation: type of the operation. Should be the name of a member of
	 *	struct &media_entity_operations.
	 *
	 * This helper function will check if @operation is not %NULL. On such case,
	 * it will issue a call to @operation\(@entity, @args\).
	 */

	#define media_entity_call(entity, operation, args...)			\
		(((entity)->ops && (entity)->ops->operation) ?			\
		 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)


Basically, the Sphinx C domain seems to be expecting a description for
"...". I didn't find any way to get rid of that.

2) a nitpick warning at v4l2-mem2mem.h:

./include/media/v4l2-mem2mem.h:339: WARNING: c:type reference target not found: queue_init


	/**
	 * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
	 *
	 * @m2m_dev: opaque pointer to the internal data to handle M2M context
	 * @drv_priv: driver's instance private data
	 * @queue_init: a callback for queue type-specific initialization function
	 * 	to be used for initializing videobuf_queues
	 *
	 * Usually called from driver's ``open()`` function.
	 */
	struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
			void *drv_priv,
			int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));

I checked the output of kernel-doc, and it looked ok. Yet, it expects
"queue_init" to be defined somehow. I suspect that this is an error at
Sphinx C domain parser.

Markus,

Could you please take a look on those?

Thanks,
Mauro

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Am 09.09.2016 um 14:08 schrieb Mauro Carvalho Chehab <mchehab@infradead.org>:

> Em Wed,  7 Sep 2016 09:12:55 +0200
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
> 
>> From: Markus Heiser <markus.heiser@darmarIT.de>
>> 
>> Hi Jon,
>> 
>> according to your remarks I fixed the first and second patch. The third patch is
>> resend unchanged;
>> 
>>> Am 06.09.2016 um 14:28 schrieb Jonathan Corbet <corbet@lwn.net>:
>>> 
>>> As others have pointed out, we generally want to hide the difference
>>> between functions and macros, so this is probably one change we don't
>>> want.  
>> 
>> I read "probably", so there might be a chance to persuade you ;)
>> 
>> I'm not a friend of *information hiding* and since the index is sorted
>> alphabetical it does no matter if the entry is 'FOO (C function)' or 'FOO (C
>> macro)'. The last one has the right information e.g. for someone how is looking
>> for a macro. FOO is a function-like macro and not a function, if the author
>> describes the macro he might use the word "macro FOO" but in the index it is
>> tagged as C function.
>> 
>> Macros and functions are totally different even if their notation looks
>> similarly. So where is the benefit of entries like 'FOO (C function)', which is
>> IMHO ambiguous.
>> 
>> I tagged the 'function-like macros index entry' patch with 'RFC' and resend it
>> within this series. If you and/or others have a different opinion, feel free to
>> drop it.
>> 
>> Thanks for review.
>> 
>> -- Markus --
>> 
>> 
>> Markus Heiser (3):
>>  doc-rst:c-domain: fix sphinx version incompatibility
>>  doc-rst:c-domain: function-like macros arguments
>>  doc-rst:c-domain: function-like macros index entry
>> 
>> Documentation/sphinx/cdomain.py | 79 +++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 76 insertions(+), 3 deletions(-)
>> 
> 
> Those patches indeed fix the issues. The arguments are now
> processed properly.
> 
> Tested-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> 
> ---
> 
> Using either this approach or my kernel-doc patch, I'm now getting
> only two warnings:
> 
> 1) at media-entity.h, even without nitpick mode:
> 
> ./include/media/media-entity.h:1053: warning: No description found for parameter '...'
> 
> This is caused by this kernel-doc tag and the corresponding macro:
> 
> 	/**
> 	 * media_entity_call - Calls a struct media_entity_operations operation on
> 	 *	an entity
> 	 *
> 	 * @entity: entity where the @operation will be called
> 	 * @operation: type of the operation. Should be the name of a member of
> 	 *	struct &media_entity_operations.
> 	 *
> 	 * This helper function will check if @operation is not %NULL. On such case,
> 	 * it will issue a call to @operation\(@entity, @args\).
> 	 */
> 
> 	#define media_entity_call(entity, operation, args...)			\
> 		(((entity)->ops && (entity)->ops->operation) ?			\
> 		 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
> 
> 
> Basically, the Sphinx C domain seems to be expecting a description for
> "...". I didn't find any way to get rid of that.
> 
> 2) a nitpick warning at v4l2-mem2mem.h:
> 
> ./include/media/v4l2-mem2mem.h:339: WARNING: c:type reference target not found: queue_init
> 
> 
> 	/**
> 	 * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
> 	 *
> 	 * @m2m_dev: opaque pointer to the internal data to handle M2M context
> 	 * @drv_priv: driver's instance private data
> 	 * @queue_init: a callback for queue type-specific initialization function
> 	 * 	to be used for initializing videobuf_queues
> 	 *
> 	 * Usually called from driver's ``open()`` function.
> 	 */
> 	struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
> 			void *drv_priv,
> 			int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));
> 
> I checked the output of kernel-doc, and it looked ok. Yet, it expects
> "queue_init" to be defined somehow. I suspect that this is an error at
> Sphinx C domain parser.
> 
> Markus,
> 
> Could you please take a look on those?

Yes, I will give it a try, but I don't know if I find the time
today.

On wich branch could I test this?

-- Markus --

> 
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-doc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Jonathan Corbet]

On Wed,  7 Sep 2016 09:12:55 +0200
Markus Heiser <markus.heiser@darmarit.de> wrote:

> according to your remarks I fixed the first and second patch. The third patch is
> resend unchanged;

OK, I've applied the first two, finally.

> > Am 06.09.2016 um 14:28 schrieb Jonathan Corbet <corbet@lwn.net>:
> >
> > As others have pointed out, we generally want to hide the difference
> > between functions and macros, so this is probably one change we don't
> > want.  
> 
> I read "probably", so there might be a chance to persuade you ;)
> 
> I'm not a friend of *information hiding* and since the index is sorted
> alphabetical it does no matter if the entry is 'FOO (C function)' or 'FOO (C
> macro)'. The last one has the right information e.g. for someone how is looking
> for a macro. FOO is a function-like macro and not a function, if the author
> describes the macro he might use the word "macro FOO" but in the index it is
> tagged as C function.

Information hiding is the only way we can maintain the kernel and stay
sane.  I have a hard time imagining why somebody would be looking for a
macro in particular; the whole idea is that they really shouldn't have to
care.  So my inclination is to leave this one out, sorry.

Thanks,

jon

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Am 16.09.2016 um 18:02 schrieb Jonathan Corbet <corbet@lwn.net>:

> On Wed,  7 Sep 2016 09:12:55 +0200
> Markus Heiser <markus.heiser@darmarit.de> wrote:
> 
>> according to your remarks I fixed the first and second patch. The third patch is
>> resend unchanged;
> 
> OK, I've applied the first two, finally.
...

> Information hiding is the only way we can maintain the kernel and stay
> sane.  I have a hard time imagining why somebody would be looking for a
> macro in particular; the whole idea is that they really shouldn't have to
> care.  So my inclination is to leave this one out, sorry.

OK, thanks.
 
-- Markus --

> 
> Thanks,
> 
> jon

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Hi Mauro, 

sorry for my late reply (so much work to do) ..

Am 09.09.2016 um 14:25 schrieb Markus Heiser <markus.heiser@darmarIT.de>:

>> Using either this approach or my kernel-doc patch, I'm now getting
>> only two warnings:
>> 
>> 1) at media-entity.h, even without nitpick mode:
>> 
>> ./include/media/media-entity.h:1053: warning: No description found for parameter '...'

FYI: This message comes from the kernel-doc parser.

>> This is caused by this kernel-doc tag and the corresponding macro:
>> 
>> 	/**
>> 	 * media_entity_call - Calls a struct media_entity_operations operation on
>> 	 *	an entity
>> 	 *
>> 	 * @entity: entity where the @operation will be called
>> 	 * @operation: type of the operation. Should be the name of a member of
>> 	 *	struct &media_entity_operations.
>> 	 *
>> 	 * This helper function will check if @operation is not %NULL. On such case,
>> 	 * it will issue a call to @operation\(@entity, @args\).
>> 	 */
>> 
>> 	#define media_entity_call(entity, operation, args...)			\
>> 		(((entity)->ops && (entity)->ops->operation) ?			\
>> 		 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
>> 
>> 
>> Basically, the Sphinx C domain seems to be expecting a description for
>> "...". I didn't find any way to get rid of that.

This is a bug in the kernel-doc parser.	The parser generates:

  .. c:function:: media_entity_call ( entity,  operation,  ...)

correct is:

  .. c:function::  media_entity_call( entity,  operation,  args...)

So both, the message and the wrong parse result comes from kernel-doc.

>> 
>> 2) a nitpick warning at v4l2-mem2mem.h:
>> 
>> ./include/media/v4l2-mem2mem.h:339: WARNING: c:type reference target not found: queue_init

FYI: this message comes from sphinx c-domain.

>> 	/**
>> 	 * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
>> 	 *
>> 	 * @m2m_dev: opaque pointer to the internal data to handle M2M context
>> 	 * @drv_priv: driver's instance private data
>> 	 * @queue_init: a callback for queue type-specific initialization function
>> 	 * 	to be used for initializing videobuf_queues
>> 	 *
>> 	 * Usually called from driver's ``open()`` function.
>> 	 */
>> 	struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>> 			void *drv_priv,
>> 			int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));
>> 
>> I checked the output of kernel-doc, and it looked ok. Yet, it expects
>> "queue_init" to be defined somehow. I suspect that this is an error at
>> Sphinx C domain parser.

Hmm, as far as I see, the output is not correct ... The output of
functions with a function pointer argument are missing the 
leading parenthesis in the function definition:

  .. c:function:: struct v4l2_m2m_ctx * v4l2_m2m_ctx_init (struct v4l2_m2m_dev * m2m_dev, void * drv_priv, int (*queue_init) (void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)

The missing parenthesis cause the error message. 

The output of the parameter description is:

  ``int (*)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) queue_init``
    a callback for queue type-specific initialization function
    to be used for initializing videobuf_queues

Correct (and IMO better to read) is:

  .. c:function:: struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev, void *drv_priv, int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq))

and the parameter description should be something like ...

   :param int (\*queue_init)(void \*priv, struct vb2_queue \*src_vq, struct vb2_queue \*dst_vq):
        a callback for queue type-specific initialization function
        to be used for initializing videobuf_queues

I tested this with my linuxdoc tools (parser) with I get no
error messages from the sphinx c-domain.

BTW: 

The parser of my linuxdoc project is more strict and spit out some 
warnings, which are not detected by the kernel-doc parser from the
kernel source tree.

For your rework on kernel-doc comments, it might be helpful to see
those messages, so I recommend to install the linuxdoc package and
do some lint.

install: https://return42.github.io/linuxdoc/install.html
lint:    https://return42.github.io/linuxdoc/cmd-line.html#kernel-lintdoc

E.g. if you want to lint your whole include/media tree type:

  kernel-lintdoc [--sloppy] include/media


-- Markus --

>> 
>> Markus,
>> 
>> Could you please take a look on those?
> 
> Yes, I will give it a try, but I don't know if I find the time
> today.
> 
> On wich branch could I test this?
> 
> -- Markus --
> 
>> 
>> Thanks,
>> Mauro

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Mauro Carvalho Chehab]

Em Mon, 19 Sep 2016 13:36:55 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> Hi Mauro, 
> 
> sorry for my late reply (so much work to do) ..
> 
> Am 09.09.2016 um 14:25 schrieb Markus Heiser <markus.heiser@darmarIT.de>:
> 
> >> Using either this approach or my kernel-doc patch, I'm now getting
> >> only two warnings:
> >> 
> >> 1) at media-entity.h, even without nitpick mode:
> >> 
> >> ./include/media/media-entity.h:1053: warning: No description found for parameter '...'  
> 
> FYI: This message comes from the kernel-doc parser.
> 
> >> This is caused by this kernel-doc tag and the corresponding macro:
> >> 
> >> 	/**
> >> 	 * media_entity_call - Calls a struct media_entity_operations operation on
> >> 	 *	an entity
> >> 	 *
> >> 	 * @entity: entity where the @operation will be called
> >> 	 * @operation: type of the operation. Should be the name of a member of
> >> 	 *	struct &media_entity_operations.
> >> 	 *
> >> 	 * This helper function will check if @operation is not %NULL. On such case,
> >> 	 * it will issue a call to @operation\(@entity, @args\).
> >> 	 */
> >> 
> >> 	#define media_entity_call(entity, operation, args...)			\
> >> 		(((entity)->ops && (entity)->ops->operation) ?			\
> >> 		 (entity)->ops->operation((entity) , ##args) : -ENOIOCTLCMD)
> >> 
> >> 
> >> Basically, the Sphinx C domain seems to be expecting a description for
> >> "...". I didn't find any way to get rid of that.  
> 
> This is a bug in the kernel-doc parser.	The parser generates:
> 
>   .. c:function:: media_entity_call ( entity,  operation,  ...)
> 
> correct is:
> 
>   .. c:function::  media_entity_call( entity,  operation,  args...)
> 
> So both, the message and the wrong parse result comes from kernel-doc.

Ok, I'll try to address it by fixing kernel-doc script.

> 
> >> 
> >> 2) a nitpick warning at v4l2-mem2mem.h:
> >> 
> >> ./include/media/v4l2-mem2mem.h:339: WARNING: c:type reference target not found: queue_init  
> 
> FYI: this message comes from sphinx c-domain.
> 
> >> 	/**
> >> 	 * v4l2_m2m_ctx_init() - allocate and initialize a m2m context
> >> 	 *
> >> 	 * @m2m_dev: opaque pointer to the internal data to handle M2M context
> >> 	 * @drv_priv: driver's instance private data
> >> 	 * @queue_init: a callback for queue type-specific initialization function
> >> 	 * 	to be used for initializing videobuf_queues
> >> 	 *
> >> 	 * Usually called from driver's ``open()`` function.
> >> 	 */
> >> 	struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
> >> 			void *drv_priv,
> >> 			int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq));
> >> 
> >> I checked the output of kernel-doc, and it looked ok. Yet, it expects
> >> "queue_init" to be defined somehow. I suspect that this is an error at
> >> Sphinx C domain parser.  
> 
> Hmm, as far as I see, the output is not correct ... The output of
> functions with a function pointer argument are missing the 
> leading parenthesis in the function definition:
> 
>   .. c:function:: struct v4l2_m2m_ctx * v4l2_m2m_ctx_init (struct v4l2_m2m_dev * m2m_dev, void * drv_priv, int (*queue_init) (void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> 
> The missing parenthesis cause the error message. 


Ah, OK! I'll kernel-doc and see what's happening here.

> 
> The output of the parameter description is:
> 
>   ``int (*)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) queue_init``
>     a callback for queue type-specific initialization function
>     to be used for initializing videobuf_queues
> 
> Correct (and IMO better to read) is:
> 
>   .. c:function:: struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev, void *drv_priv, int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq))
> 
> and the parameter description should be something like ...
> 
>    :param int (\*queue_init)(void \*priv, struct vb2_queue \*src_vq, struct vb2_queue \*dst_vq):
>         a callback for queue type-specific initialization function
>         to be used for initializing videobuf_queues

I guess the better would be to strip the parameter type and output
it as:
	queue_init
		a callback for queue type-specific initialization function
		to be used for initializing videobuf_queues

As I pointed before, the point is that such argument can easily have
more than 80 columns, with would cause troubles with LaTeX output,
as LaTeX doesn't break long verbatim text on multiple lines.

I submitted one patch fixing it. Not sure if it got merged by Jon
or not.

> 
> I tested this with my linuxdoc tools (parser) with I get no
> error messages from the sphinx c-domain.
> 
> BTW: 
> 
> The parser of my linuxdoc project is more strict and spit out some 
> warnings, which are not detected by the kernel-doc parser from the
> kernel source tree.
> 
> For your rework on kernel-doc comments, it might be helpful to see
> those messages, so I recommend to install the linuxdoc package and
> do some lint.
> 
> install: https://return42.github.io/linuxdoc/install.html
> lint:    https://return42.github.io/linuxdoc/cmd-line.html#kernel-lintdoc

Interesting! Yeah, it caught a lot more errors ;)

If I understood it right, I could do something like:

diff --git a/Documentation/media/conf_nitpick.py b/Documentation/media/conf_nitpick.py
index 480d548af670..2de603871536 100644
--- a/Documentation/media/conf_nitpick.py
+++ b/Documentation/media/conf_nitpick.py
@@ -107,3 +107,9 @@ nitpick_ignore = [
 
     ("c:type", "v4l2_m2m_dev"),
 ]
+
+
+extensions.append("linuxdoc.rstKernelDoc")
+extensions.append("linuxdoc.rstFlatTable")
+extensions.append("linuxdoc.kernel_include")
+extensions.append("linuxdoc.manKernelDoc")

Right? It would be good to do some sort of logic on the
above for it to automatically include it, if linuxdoc is
present, otherwise print a warning and do "just" the normal
Sphinx tests.

> 
> E.g. if you want to lint your whole include/media tree type:
> 
>   kernel-lintdoc [--sloppy] include/media

Yeah, running it manually is another way, although I prefer to have
it done via a Makefile target, and doing only for the files that
are currently inside a Sphinx rst file (at least on a first moment).

Thanks,
Mauro

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Am 19.09.2016 um 17:00 schrieb Mauro Carvalho Chehab <mchehab@infradead.org>:

>> Hmm, as far as I see, the output is not correct ... The output of
>> functions with a function pointer argument are missing the 
>> leading parenthesis in the function definition:
>> 
>>  .. c:function:: struct v4l2_m2m_ctx * v4l2_m2m_ctx_init (struct v4l2_m2m_dev * m2m_dev, void * drv_priv, int (*queue_init) (void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>> 
>> The missing parenthesis cause the error message. 
> 
> 
> Ah, OK! I'll kernel-doc and see what's happening here.
> 
>> 
>> The output of the parameter description is:
>> 
>>  ``int (*)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) queue_init``
>>    a callback for queue type-specific initialization function
>>    to be used for initializing videobuf_queues
>> 
>> Correct (and IMO better to read) is:
>> 
>>  .. c:function:: struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev, void *drv_priv, int (*queue_init)(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq))
>> 
>> and the parameter description should be something like ...
>> 
>>   :param int (\*queue_init)(void \*priv, struct vb2_queue \*src_vq, struct vb2_queue \*dst_vq):
>>        a callback for queue type-specific initialization function
>>        to be used for initializing videobuf_queues
> 
> I guess the better would be to strip the parameter type and output
> it as:
> 	queue_init
> 		a callback for queue type-specific initialization function
> 		to be used for initializing videobuf_queues

Good point!

> As I pointed before, the point is that such argument can easily have
> more than 80 columns, with would cause troubles with LaTeX output,
> as LaTeX doesn't break long verbatim text on multiple lines.
> 
> I submitted one patch fixing it. Not sure if it got merged by Jon
> or not.

Ups, I might have overseen this patch .. as Jon said, its hard to
follow you ;)

I tested the above with Jon's docs-next, so it seems your patch is
not yet applied. Could you send me a link for this patch? (sorry,
I can't find it).


>> I tested this with my linuxdoc tools (parser) with I get no
>> error messages from the sphinx c-domain.
>> 
>> BTW: 
>> 
>> The parser of my linuxdoc project is more strict and spit out some 
>> warnings, which are not detected by the kernel-doc parser from the
>> kernel source tree.
>> 
>> For your rework on kernel-doc comments, it might be helpful to see
>> those messages, so I recommend to install the linuxdoc package and
>> do some lint.
>> 
>> install: https://return42.github.io/linuxdoc/install.html
>> lint:    https://return42.github.io/linuxdoc/cmd-line.html#kernel-lintdoc
> 
> Interesting! Yeah, it caught a lot more errors ;)
> 
> If I understood it right, I could do something like:
> 
> diff --git a/Documentation/media/conf_nitpick.py b/Documentation/media/conf_nitpick.py
> index 480d548af670..2de603871536 100644
> --- a/Documentation/media/conf_nitpick.py
> +++ b/Documentation/media/conf_nitpick.py
> @@ -107,3 +107,9 @@ nitpick_ignore = [
> 
>     ("c:type", "v4l2_m2m_dev"),
> ]
> +
> +
> +extensions.append("linuxdoc.rstKernelDoc")
> +extensions.append("linuxdoc.rstFlatTable")
> +extensions.append("linuxdoc.kernel_include")
> +extensions.append("linuxdoc.manKernelDoc")
> 
> Right?

No ;)

> It would be good to do some sort of logic on the
> above for it to automatically include it, if linuxdoc is
> present, otherwise print a warning and do "just" the normal
> Sphinx tests.

The intention is; with installing the linuxdoc project you get
some nice command line tools, like lint for free and if you want
to see how the linuxdoc project compiles your kernel documentation
and how e.g. man pages are build or what warnings are spit, you
have to **replace** the extensions from the kernel's source with
the one from the linuxdoc project.

This is done by patching the build scripts as described in:

  https://return42.github.io/linuxdoc/linux.html

FYI: I updated the documentation of the linuxdoc project.

In this project I develop and maintain "future" concepts like
man-page builder and the py-version of the kernel-doc parser. 
Vice versa, every improvement I see on kernel's source tree is
merged into this project.

This project is also used by my POC at:

* http://return42.github.io/sphkerneldoc/

E.g. it builds the documentation of the complete kernel sources

* http://return42.github.io/sphkerneldoc/linux_src_doc/index.html

the last one is also my test-case to find regression when I change
something / running against the whole source tree and compare the
result to the versioned reST files at 

* https://github.com/return42/sphkerneldoc/tree/master/doc/linux_src_doc

-- Markus --

>> E.g. if you want to lint your whole include/media tree type:
>> 
>>  kernel-lintdoc [--sloppy] include/media
> 
> Yeah, running it manually is another way, although I prefer to have
> it done via a Makefile target, and doing only for the files that
> are currently inside a Sphinx rst file (at least on a first moment).
> 
> Thanks,
> Mauro

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Jonathan Corbet]

On Tue, 20 Sep 2016 20:56:35 +0200
Markus Heiser <markus.heiser@darmarit.de> wrote:

> > I submitted one patch fixing it. Not sure if it got merged by Jon
> > or not.  
> 
> Ups, I might have overseen this patch .. as Jon said, its hard to
> follow you ;)
> 
> I tested the above with Jon's docs-next, so it seems your patch is
> not yet applied. Could you send me a link for this patch? (sorry,
> I can't find it).

Send again, please?  I'll add it to the pile of other stuff, and try not
to lose it again...:)

jon

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Mauro Carvalho Chehab]

Em Tue, 20 Sep 2016 13:00:33 -0600
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Tue, 20 Sep 2016 20:56:35 +0200
> Markus Heiser <markus.heiser@darmarit.de> wrote:
> 
> > > I submitted one patch fixing it. Not sure if it got merged by Jon
> > > or not.  
> > 
> > Ups, I might have overseen this patch .. as Jon said, its hard to
> > follow you ;)
> > 
> > I tested the above with Jon's docs-next, so it seems your patch is
> > not yet applied. Could you send me a link for this patch? (sorry,
> > I can't find it).
> 
> Send again, please?  I'll add it to the pile of other stuff, and try not
> to lose it again...:)

Gah, there are so many patches that I'm also confused whether I sent something
or just dreamed about sending it :)

I actually sent a patch doing this on a /47 patch series, but only
for macros:

	Subject: [PATCH 01/47] kernel-doc: ignore arguments on macro definitions

I was thinking on doing the same for functions, but didn't actually
submitted such patch.

Yet, it seems more coherent, IMHO, to use use same approach for both C
functions and macros: presenting just the name instead of printing the
arguments.

I'll work on it and submit, likely tomorrow.

Thanks,
Mauro

Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Mauro Carvalho Chehab]

Em Tue, 20 Sep 2016 20:56:35 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:

> > If I understood it right, I could do something like:
> > 
> > diff --git a/Documentation/media/conf_nitpick.py b/Documentation/media/conf_nitpick.py
> > index 480d548af670..2de603871536 100644
> > --- a/Documentation/media/conf_nitpick.py
> > +++ b/Documentation/media/conf_nitpick.py
> > @@ -107,3 +107,9 @@ nitpick_ignore = [
> > 
> >     ("c:type", "v4l2_m2m_dev"),
> > ]
> > +
> > +
> > +extensions.append("linuxdoc.rstKernelDoc")
> > +extensions.append("linuxdoc.rstFlatTable")
> > +extensions.append("linuxdoc.kernel_include")
> > +extensions.append("linuxdoc.manKernelDoc")
> > 
> > Right?  
> 
> No ;)

> > It would be good to do some sort of logic on the
> > above for it to automatically include it, if linuxdoc is
> > present, otherwise print a warning and do "just" the normal
> > Sphinx tests.  
> 
> The intention is; with installing the linuxdoc project you get
> some nice command line tools, like lint for free and if you want
> to see how the linuxdoc project compiles your kernel documentation
> and how e.g. man pages are build or what warnings are spit, you
> have to **replace** the extensions from the kernel's source with
> the one from the linuxdoc project.
> 
> This is done by patching the build scripts as described in:
> 
>   https://return42.github.io/linuxdoc/linux.html
> 
> FYI: I updated the documentation of the linuxdoc project.

Hmm... It would be a way better to just do something like the
above patch, specially since a conf_lint.py could have the
modified conf_nitpick.py, and avoiding touching at the
tree. The need of making a patch to use it, touching at the
building system prevents it to be called for every applied
patch that would touch on a documented header file.

I used the above to detect some cut-and-paste issues with some
documentation.

Yet, from what I saw, the parsers of lint still need some work,
as it didn't parse some stuff at the media headers that seem ok.

> In this project I develop and maintain "future" concepts like
> man-page builder and the py-version of the kernel-doc parser. 

The new parser seems to have some bugs, like those:

$ kernel-lintdoc include/media/v4l2-ctrls.h
include/media/v4l2-ctrls.h:106 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_notify_fnc' in the comment.
...
include/media/v4l2-ctrls.h:605 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_filter' in the comment.
...
include/media/v4l2-ctrls.h:809 [kernel-doc WARN] : can't understand function proto: 'const char * const *v4l2_ctrl_get_menu(u32 id);'
...

in this case, both typedefs were defined. The prototype for
v4l2_ctrl_get_menu() is also valid.

Anyway, it helped to get some real troubles. Thanks!


> Vice versa, every improvement I see on kernel's source tree is
> merged into this project.
> 
> This project is also used by my POC at:
> 
> * http://return42.github.io/sphkerneldoc/
> 
> E.g. it builds the documentation of the complete kernel sources
> 
> * http://return42.github.io/sphkerneldoc/linux_src_doc/index.html
> 
> the last one is also my test-case to find regression when I change
> something / running against the whole source tree and compare the
> result to the versioned reST files at 
> 
> * https://github.com/return42/sphkerneldoc/tree/master/doc/linux_src_doc
> 
> -- Markus --
> 
> >> E.g. if you want to lint your whole include/media tree type:
> >> 
> >>  kernel-lintdoc [--sloppy] include/media  
> > 
> > Yeah, running it manually is another way, although I prefer to have
> > it done via a Makefile target, and doing only for the files that
> > are currently inside a Sphinx rst file (at least on a first moment).
> > 
> > Thanks,
> > Mauro  
> 



Thanks,
Mauro

kernel-lintdoc parser - was: Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Mauro Carvalho Chehab]

Hi Markus,

Em Thu, 22 Sep 2016 09:08:50 -0300
Mauro Carvalho Chehab <mchehab@infradead.org> escreveu:

> Em Tue, 20 Sep 2016 20:56:35 +0200
> Markus Heiser <markus.heiser@darmarit.de> escreveu:
> 

> The new parser seems to have some bugs, like those:
> 
> $ kernel-lintdoc include/media/v4l2-ctrls.h
> include/media/v4l2-ctrls.h:106 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_notify_fnc' in the comment.
> ...
> include/media/v4l2-ctrls.h:605 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_filter' in the comment.
> ...
> include/media/v4l2-ctrls.h:809 [kernel-doc WARN] : can't understand function proto: 'const char * const *v4l2_ctrl_get_menu(u32 id);'
> ...

I ran the kernel-lintdoc with:
	for i in $(git grep kernel-doc Documentation/media/kapi/|cut -d: -f4); do kernel-lintdoc --sloppy $i; done

and I have a few comments:

1) instead of printing the full patch, it would be good to print the
relative patch, as this makes easier to paste the errors on e-mails
and on patches.

2) Parsing of embedded structs/unions

On some headers like dvb_frontend.h, we have unamed structs and enums
inside structs:

struct dtv_frontend_properties {
...
	struct {
	    u8			segment_count;
	    enum fe_code_rate	fec;
	    enum fe_modulation	modulation;
	    u8			interleaving;
	} layer[3];
...
};

The fields of the embedded struct are described as:

 * @layer:		ISDB per-layer data (only ISDB standard)
 * @layer.segment_count: Segment Count;
 * @layer.fec:		per layer code rate;
 * @layer.modulation:	per layer modulation;
 * @layer.interleaving:	 per layer interleaving.

kernel-lintdoc didn't like that:

	drivers/media/dvb-core/dvb_frontend.h:513 :ERROR: duplicate parameter definition 'layer'
	drivers/media/dvb-core/dvb_frontend.h:514 :ERROR: duplicate parameter definition 'layer'
	drivers/media/dvb-core/dvb_frontend.h:515 :ERROR: duplicate parameter definition 'layer'
	drivers/media/dvb-core/dvb_frontend.h:516 :ERROR: duplicate parameter definition 'layer'

2) it complains about function typedefs:

	drivers/media/dvb-core/demux.h:251 :WARN: typedef of function pointer not marked as typdef, use: 'typedef dmx_ts_cb' in the comment.
	drivers/media/dvb-core/demux.h:292 :WARN: typedef of function pointer not marked as typdef, use: 'typedef dmx_section_cb' in the comment.
	include/media/v4l2-ioctl.h:677 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_kioctl' in the comment.
	include/media/v4l2-ctrls.h:106 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_notify_fnc' in the comment.
	include/media/v4l2-ctrls.h:606 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_filter' in the comment.
	include/media/v4l2-dv-timings.h:39 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_check_dv_timings_fnc' in the comment.
	include/media/v4l2-dv-timings.h:39 :WARN: typedef of function pointer used uncommon code style: 'typedef bool v4l2_check_dv_timings_fnc(const struct v4l2_dv_timings *t, void *handle);'
	include/media/videobuf2-core.h:877 :WARN: typedef of function pointer not marked as typdef, use: 'typedef vb2_thread_fnc' in the comment.

3) this is actually a more complex problem: how to represent returned values
from the function callbacks. Maybe we'll need to patch kernel-doc. Right now,
it complains with:

	drivers/media/dvb-core/demux.h:397 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:415 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:431 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:444 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:462 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:475 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:491 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:507 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:534 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:542 :WARN: duplicate section name 'It returns'
	drivers/media/dvb-core/demux.h:552 :WARN: duplicate section name 'It returns'

4) Parse errors.

Those seem to be caused by some errors at the parser:

	include/media/v4l2-ctrls.h:809 :WARN: can't understand function proto: 'const char * const *v4l2_ctrl_get_menu(u32 id);'
	include/media/v4l2-dev.h:173 :WARN: no description found for parameter 'valid_ioctls\[BITS_TO_LONGS(BASE_VIDIOC_PRIVATE)\]'
	include/media/v4l2-dev.h:173 :WARN: no description found for parameter 'disable_locking\[BITS_TO_LONGS(BASE_VIDIOC_PRIVATE)\]'
	include/media/videobuf2-core.h:555 :ERROR: can't parse struct!


Thanks,
Mauro

Re: kernel-lintdoc parser - was: Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Hi Mauro,

thanks a lot for your tests and inspirations ...

Am 22.09.2016 um 14:35 schrieb Mauro Carvalho Chehab <mchehab@infradead.org>:

> Hi Markus,
> 
> Em Thu, 22 Sep 2016 09:08:50 -0300
> Mauro Carvalho Chehab <mchehab@infradead.org> escreveu:
> 
>> Em Tue, 20 Sep 2016 20:56:35 +0200
>> Markus Heiser <markus.heiser@darmarit.de> escreveu:
>> 
> 
>> The new parser seems to have some bugs, like those:
>> 
>> $ kernel-lintdoc include/media/v4l2-ctrls.h
>> include/media/v4l2-ctrls.h:106 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_notify_fnc' in the comment.
>> ...
>> include/media/v4l2-ctrls.h:605 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_filter' in the comment.
>> ...
>> include/media/v4l2-ctrls.h:809 [kernel-doc WARN] : can't understand function proto: 'const char * const *v4l2_ctrl_get_menu(u32 id);'
>> ...
> 
> I ran the kernel-lintdoc with:
> 	for i in $(git grep kernel-doc Documentation/media/kapi/|cut -d: -f4); do kernel-lintdoc --sloppy $i; done
> 
> and I have a few comments:
> 
> 1) instead of printing the full patch, it would be good to print the
> relative patch, as this makes easier to paste the errors on e-mails
> and on patches.

relative patch? ... I think you mean relative path, if so: 

I can add an option like "--abspath", not a big deal
but whats about calling the lint with $(pwd)/$i ::

 for i in $(git grep kernel-doc Documentation/media/kapi/|cut -d: -f4); do kernel-lintdoc --sloppy $(pwd)/$i; done

.. fits this solution to your use-case?


> 2) Parsing of embedded structs/unions
> 
> On some headers like dvb_frontend.h, we have unamed structs and enums
> inside structs:
> 
> struct dtv_frontend_properties {
> ...
> 	struct {
> 	    u8			segment_count;
> 	    enum fe_code_rate	fec;
> 	    enum fe_modulation	modulation;
> 	    u8			interleaving;
> 	} layer[3];
> ...
> };
> 
> The fields of the embedded struct are described as:
> 
> * @layer:		ISDB per-layer data (only ISDB standard)
> * @layer.segment_count: Segment Count;
> * @layer.fec:		per layer code rate;
> * @layer.modulation:	per layer modulation;
> * @layer.interleaving:	 per layer interleaving.
> 
> kernel-lintdoc didn't like that:
> 
> 	drivers/media/dvb-core/dvb_frontend.h:513 :ERROR: duplicate parameter definition 'layer'
> 	drivers/media/dvb-core/dvb_frontend.h:514 :ERROR: duplicate parameter definition 'layer'
> 	drivers/media/dvb-core/dvb_frontend.h:515 :ERROR: duplicate parameter definition 'layer'
> 	drivers/media/dvb-core/dvb_frontend.h:516 :ERROR: duplicate parameter definition 'layer'

Hah .. fixed this yesterday ;-)

  https://github.com/return42/linuxdoc/commit/531df6dd7728393f447b1a4b3215b96687d02ba2

I analyzed this yesterday and haven't had time to report it,
so I will do it now:

   This is also broken in the kernel-doc (perl) parser
   .. are you able to fix it? 

I can give it I try, but I have no extensive test case for the
perl version and perl is a bid harder for me. So sometimes I'am
a bit scary about patching the kernel-doc perl script.
(as I said before, for the py version I test against the
whole source and compare/versionize the resulted reST)

What I tested:

  ./scripts/kernel-doc -rst -function dtv_frontend_properties drivers/media/dvb-core/dvb_frontend.h

for "layer" it outputs:

    ``layer[3]``
      per layer interleaving.

Read my commit message above .. the description block is taken
from " @layer.interleaving:" instead from "@layer:".


> 2) it complains about function typedefs:
> 
> 	drivers/media/dvb-core/demux.h:251 :WARN: typedef of function pointer not marked as typdef, use: 'typedef dmx_ts_cb' in the comment.
> 	drivers/media/dvb-core/demux.h:292 :WARN: typedef of function pointer not marked as typdef, use: 'typedef dmx_section_cb' in the comment.
> 	include/media/v4l2-ioctl.h:677 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_kioctl' in the comment.
> 	include/media/v4l2-ctrls.h:106 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_notify_fnc' in the comment.
> 	include/media/v4l2-ctrls.h:606 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_ctrl_filter' in the comment.
> 	include/media/v4l2-dv-timings.h:39 :WARN: typedef of function pointer not marked as typdef, use: 'typedef v4l2_check_dv_timings_fnc' in the comment.
> 	include/media/v4l2-dv-timings.h:39 :WARN: typedef of function pointer used uncommon code style: 'typedef bool v4l2_check_dv_timings_fnc(const struct v4l2_dv_timings *t, void *handle);'
> 	include/media/videobuf2-core.h:877 :WARN: typedef of function pointer not marked as typdef, use: 'typedef vb2_thread_fnc' in the comment.

Thanks for reporting this ... fixed it:

https://github.com/return42/linuxdoc/commit/9228f2128dba967519fd8f2cdeb2c2202caad84d

> 3) this is actually a more complex problem: how to represent returned values
> from the function callbacks. Maybe we'll need to patch kernel-doc. Right now,
> it complains with:
> 
> 	drivers/media/dvb-core/demux.h:397 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:415 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:431 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:444 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:462 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:475 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:491 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:507 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:534 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:542 :WARN: duplicate section name 'It returns'
> 	drivers/media/dvb-core/demux.h:552 :WARN: duplicate section name 'It returns'

Hmm, IMO we should keep the kernel-doc markup simple.
Sub-sections in parameter descriptions are not provided
and we should not change this.
This in mind, the above WARN messages are inappropriate.
I fixed the parser.

https://github.com/return42/linuxdoc/commit/6b664f537adc7970baffc8dae1ecf97c601ac7f9


> 4) Parse errors.
> 
> Those seem to be caused by some errors at the parser:
> 
> 	include/media/v4l2-ctrls.h:809 :WARN: can't understand function proto: 'const char * const *v4l2_ctrl_get_menu(u32 id);'

Argh, there was a type in the regexpr / fixed:

  https://github.com/return42/linuxdoc/commit/dcf91bb2220c64135a2da7df866d06c126fb103e


> 	include/media/v4l2-dev.h:173 :WARN: no description found for parameter 'valid_ioctls\[BITS_TO_LONGS(BASE_VIDIOC_PRIVATE)\]'
> 	include/media/v4l2-dev.h:173 :WARN: no description found for parameter 'disable_locking\[BITS_TO_LONGS(BASE_VIDIOC_PRIVATE)\]'
> 	include/media/videobuf2-core.h:555 :ERROR: can't parse struct!
> 

Argh, another typo .. fixed:

 https://github.com/return42/linuxdoc/commit/2947952d3fce17367193a3511349312f7a75ff04

BTW: I fixed some more issues (see last changelogs).

FYI: if you made a pip install, then update with:

  pip install --upgrade git+http://github.com/return42/linuxdoc.git

Ok let me say again, thanks for reporting all this, if you find more
please inform me.

Now I'am to tired, but I hope tomorrow I have the time to think
about your last mail: using lint in conf_nitpick.py

-- Markus --

> 
> Thanks,
> Mauro

Re: kernel-lintdoc parser - was: Re: [PATCH v2 0/3] doc-rst:c-domain: fix some issues in the c-domain

[Markus Heiser]

Am 22.09.2016 um 14:35 schrieb Mauro Carvalho Chehab <mchehab@infradead.org>:

> Hi Markus,
> 3) this is actually a more complex problem: how to represent returned values
> from the function callbacks. Maybe we'll need to patch kernel-doc.

This might be a solution for dense kernel-doc comments where you
want to have paragraph and lists in parameter descriptions:

https://github.com/return42/linuxdoc/commit/9bfb8a59326677a819f62cb16f3ffacc8b244af1

--M--