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

2016-09-22 Thread Markus Heiser 

Am 22.09.2016 um 14:35 schrieb Mauro Carvalho Chehab :

> 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--


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

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

2016-09-22 Thread Markus Heiser 
Hi Mauro,

thanks a lot for your tests and inspirations ...

Am 22.09.2016 um 14:35 schrieb Mauro Carvalho Chehab :

> Hi Markus,
> 
> Em Thu, 22 Sep 2016 09:08:50 -0300
> Mauro Carvalho Chehab  escreveu:
> 
>> Em Tue, 20 Sep 2016 20:56:35 +0200
>> Markus Heiser  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:

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

2016-09-22 Thread Mauro Carvalho Chehab 
Hi Markus,

Em Thu, 22 Sep 2016 09:08:50 -0300
Mauro Carvalho Chehab  escreveu:

> Em Tue, 20 Sep 2016 20:56:35 +0200
> Markus Heiser  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

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

2016-09-22 Thread Mauro Carvalho Chehab 
Em Tue, 20 Sep 2016 20:56:35 +0200
Markus Heiser  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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-20 Thread Mauro Carvalho Chehab 
Em Tue, 20 Sep 2016 13:00:33 -0600
Jonathan Corbet  escreveu:

> On Tue, 20 Sep 2016 20:56:35 +0200
> Markus Heiser  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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-20 Thread Jonathan Corbet 
On Tue, 20 Sep 2016 20:56:35 +0200
Markus Heiser  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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-20 Thread Markus Heiser 

Am 19.09.2016 um 17:00 schrieb Mauro Carvalho Chehab :

>> 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

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

2016-09-19 Thread Mauro Carvalho Chehab 
Em Mon, 19 Sep 2016 13:36:55 +0200
Markus Heiser  escreveu:

> Hi Mauro, 
> 
> sorry for my late reply (so much work to do) ..
> 
> Am 09.09.2016 um 14:25 schrieb Markus Heiser :
> 
> >> 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 _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 
>

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

2016-09-19 Thread Markus Heiser 
Hi Mauro, 

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

Am 09.09.2016 um 14:25 schrieb Markus Heiser :

>> 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 _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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-17 Thread Markus Heiser 

Am 16.09.2016 um 18:02 schrieb Jonathan Corbet :

> On Wed,  7 Sep 2016 09:12:55 +0200
> Markus Heiser  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

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-16 Thread Jonathan Corbet 
On Wed,  7 Sep 2016 09:12:55 +0200
Markus Heiser  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 :
> >
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-09 Thread Markus Heiser 

Am 09.09.2016 um 14:08 schrieb Mauro Carvalho Chehab :

> Em Wed,  7 Sep 2016 09:12:55 +0200
> Markus Heiser  escreveu:
> 
>> From: Markus Heiser 
>> 
>> 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 :
>>> 
>>> 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 
> 
> ---
> 
> 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 _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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@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

2016-09-09 Thread Mauro Carvalho Chehab 
Em Wed,  7 Sep 2016 09:12:55 +0200
Markus Heiser  escreveu:

> From: Markus Heiser 
> 
> 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 :
> >
> > 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 

---

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 _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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html