Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1413443 > unrolled thread

Re: [PATCH 00/10] Documentation/Sphinx

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 00/10] Documentation/Sphinx Jonathan Corbet <corbet@lwn.net> - 2016-06-03 22:20 +0200
    Re: [PATCH 00/10] Documentation/Sphinx Jonathan Corbet <corbet@lwn.net> - 2016-06-03 22:30 +0200
      Re: [PATCH 00/10] Documentation/Sphinx Jani Nikula <jani.nikula@intel.com> - 2016-06-04 15:10 +0200
    Re: [PATCH 00/10] Documentation/Sphinx Daniel Vetter <daniel.vetter@ffwll.ch> - 2016-06-03 22:30 +0200
    Re: [PATCH 00/10] Documentation/Sphinx Jani Nikula <jani.nikula@intel.com> - 2016-06-04 15:00 +0200

#1413443 — Re: [PATCH 00/10] Documentation/Sphinx

[So I'm finally trying to get into this for real, hopefully I won't be
interrupted too many times...expect a few mails as I catch up.]

On Fri, 20 May 2016 16:39:31 +0300
Jani Nikula <jani.nikula@intel.com> wrote:

> There are a few tradeoffs, of course. First, this requires that the
> EXPORT_SYMBOL markers are placed immediately after the function being
> exported, as kernel-doc will only look at one file at a time. This is
> the recommendation anyway. 

As I understand it, the technical reasons that kept some markers in
separate files should no longer be relevant, so this is probably OK.  It
would be nice to have a sense for how many sites need to be fixed.

> Second, we lose support for the !C docproc directive to check
> that all kernel-doc comments in a file are used. This is probably
> something we'd like to have back in the future, but at this time I think
> it's an acceptable tradeoff wrt the gains.

This is maybe a job for a separate tool.  A related issue is the (fairly
frequent) "oh look, none of the comments in $FILE are being used"
realization that seems to happen fairly often.  It would be nice to check
for that, but that's going to be hard to shoehorn into Sphinx.

jon

[toc] | [next] | [standalone]

#1413444

On Fri, 3 Jun 2016 22:24:03 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> > This is maybe a job for a separate tool.  A related issue is the (fairly
> > frequent) "oh look, none of the comments in $FILE are being used"
> > realization that seems to happen fairly often.  It would be nice to check
> > for that, but that's going to be hard to shoehorn into Sphinx.  
> 
> I think much more valuable would be a tool that checks whether
> comments are pulled in anywhere (for a given source file), over the
> entire tree. Very often entire subsystems carg-cult kernel-doc, but
> never use it in a .tmpl, which means 0-day won't notice, and neither
> anyone else.

That is kind of what I was trying to get at, yes :)

jon

[toc] | [prev] | [next] | [standalone]

#1413750

On Fri, 03 Jun 2016, Jonathan Corbet <corbet@lwn.net> wrote:
> On Fri, 3 Jun 2016 22:24:03 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
>> > This is maybe a job for a separate tool.  A related issue is the (fairly
>> > frequent) "oh look, none of the comments in $FILE are being used"
>> > realization that seems to happen fairly often.  It would be nice to check
>> > for that, but that's going to be hard to shoehorn into Sphinx.  
>> 
>> I think much more valuable would be a tool that checks whether
>> comments are pulled in anywhere (for a given source file), over the
>> entire tree. Very often entire subsystems carg-cult kernel-doc, but
>> never use it in a .tmpl, which means 0-day won't notice, and neither
>> anyone else.
>
> That is kind of what I was trying to get at, yes :)

For the 0-day, I've already got a preliminary kernel-doc-rst-lint
script, which can be used as a checker in the kernel build. This would
catch all kernel-doc comments, whether they're included to documentation
or not, and complain about kernel-doc and rst issues. It won't be as
comprehensive as a full Sphinx build in terms of validation, but it'll
cover more kernel-doc comments. It should catch the silly errors early.

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1413445

On Fri, Jun 3, 2016 at 10:16 PM, Jonathan Corbet <corbet@lwn.net> wrote:
>
>> Second, we lose support for the !C docproc directive to check
>> that all kernel-doc comments in a file are used. This is probably
>> something we'd like to have back in the future, but at this time I think
>> it's an acceptable tradeoff wrt the gains.
>
> This is maybe a job for a separate tool.  A related issue is the (fairly
> frequent) "oh look, none of the comments in $FILE are being used"
> realization that seems to happen fairly often.  It would be nice to check
> for that, but that's going to be hard to shoehorn into Sphinx.

I think much more valuable would be a tool that checks whether
comments are pulled in anywhere (for a given source file), over the
entire tree. Very often entire subsystems carg-cult kernel-doc, but
never use it in a .tmpl, which means 0-day won't notice, and neither
anyone else.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[toc] | [prev] | [next] | [standalone]

#1413748

On Fri, 03 Jun 2016, Jonathan Corbet <corbet@lwn.net> wrote:
> [So I'm finally trying to get into this for real, hopefully I won't be
> interrupted too many times...expect a few mails as I catch up.]
>
> On Fri, 20 May 2016 16:39:31 +0300
> Jani Nikula <jani.nikula@intel.com> wrote:
>
>> There are a few tradeoffs, of course. First, this requires that the
>> EXPORT_SYMBOL markers are placed immediately after the function being
>> exported, as kernel-doc will only look at one file at a time. This is
>> the recommendation anyway. 
>
> As I understand it, the technical reasons that kept some markers in
> separate files should no longer be relevant, so this is probably OK.  It
> would be nice to have a sense for how many sites need to be fixed.

Actually, mostly this is not a problem due to EXPORT_SYMBOL placement,
but rather due to kernel-doc comments being placed in header files above
function declarations while EXPORT_SYMBOL is where it should be next to
the function definition.

I don't think we can force people to move the kernel-doc comments for
exported functions from header files next to the function definitions.

The straightforward fix to this is to add an optional filename parameter
to the kernel-doc extension :export: argument, to pass additional files
to kernel-doc where to look for the EXPORT_SYMBOLs. For example:

.. kernel-doc:: include/drm/foo.h
   :export: drivers/gpu/drm/foo/foo.c drivers/gpu/drm/foo/bar.c

This would instruct kernel-doc to extract documentation from
include/drm/foo.h for all functions that have been exported using
EXPORT_SYMBOL (or _GPL) in include/drm/foo.h, drivers/gpu/drm/foo/foo.c,
or drivers/gpu/drm/foo/bar.c.

We have something along these lines in docproc already with the !D
directive, so nothing altogether surprising.

If my quick grep-fu serves me right, there are about a thousand exported
symbols with kernel-doc comments in the headers. It's a ballpark
figure. They come in batches; a small fraction of that many filenames in
a fraction of the :export: statements would cover most of them.

Before this fix, the workaround is to name the functions with
:functions: argument instead of using :export:.

I'm hoping this is not a blocker for merging the series. If the proposed
fix is acceptable, I'll get it done before v4.8.

BR,
Jani.


The ugly greps:

$ git grep "^EXPORT_SYMBOL" | sed 's/^[^(]*(\([a-zA-Z0-9_]*\)).*/\1/' | sort > exports
$ git grep -h -A1 "^/\*\*$" -- *.h | grep -v "^\(/\*\*\|--\)" | sed 's/^ \*[         ]*\([a-zA-Z0-9_][a-zA-Z0-9_]*\).*/\1/;' | sort > comments
$ comm -1 -2 comments exports | wc -l
952




-- 
Jani Nikula, Intel Open Source Technology Center

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web