These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely.
// <smpl> @initialize:ocaml@ @@
let tbl = ref [] let fnstart = ref [] let success = Hashtbl.create 101 let thefile = ref "" let parsed = ref [] let nea = ref []
let parse file = thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1; let i = open_in file in let startline = ref 0 in let fn = ref "" in let ids = ref [] in let rec inside n = let l = input_line i in let n = n + 1 in match Str.split_delim (Str.regexp_string "*/") l with before::after::_ -> (if not (!fn = "") then tbl := (!startline,n,!fn,List.rev !ids)::!tbl); startline := 0; fn := ""; ids := []; outside n | _ -> (match Str.split (Str.regexp "[ \t]+") l with "*"::name::rest -> let len = String.length name in (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()" then fn := String.sub name 0 (len-2) else if !fn = "" && (not (rest = [])) && List.hd rest = "-" then if String.get name (len-1) = ':' then fn := String.sub name 0 (len-1) else fn := name else if not(!fn = "") && len > 2 && String.get name 0 = '@' && String.get name (len-1) = ':' then ids := (String.sub name 1 (len-2)) :: !ids); | _ -> ()); inside n and outside n = let l = input_line i in let n = n + 1 in if String.length l > 2 && String.sub l 0 3 = "/**" then begin startline := n; inside n end else outside n in try outside 0 with End_of_file -> ()
let hashadd tbl k v = let cell = try Hashtbl.find tbl k with Not_found -> let cell = ref [] in Hashtbl.add tbl k cell; cell in cell := v :: !cell
@script:ocaml@ @@
tbl := []; fnstart := []; Hashtbl.clear success; parsed := []; nea := []; parse (List.hd (Coccilib.files()))
@r@ identifier f; position p; @@
f@p(...) { ... }
@script:ocaml@ p << r.p; f << r.f; @@
parsed := f :: !parsed; fnstart := (List.hd p).line :: !fnstart
@param@ identifier f; type T; identifier i; parameter list[n] ps; parameter list[n1] ps1; position p; @@
f@p(ps,T i,ps1) { ... }
@script:ocaml@ @@
tbl := List.rev (List.sort compare !tbl)
@script:ocaml@ p << param.p; f << param.f; @@
let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if not (String.lowercase f = String.lowercase nm) then Printf.printf "%s:%d %s doesn't match preceding comment: %s\n" !thefile myline f nm); true end else false) !tbl in ()
@script:ocaml@ p << param.p; n << param.n; n1 << param.n1; i << param.i; f << param.f; @@
let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if List.mem i ids then hashadd success (st,fn,nm) i); (if ids = [] (* arg list seems not obligatory *) then () else if not (List.mem i ids) then Printf.printf "%s:%d %s doesn't appear in ids: %s\n" !thefile myline i (String.concat " " ids) else if List.length ids <= n || List.length ids <= n1 then (if not (List.mem f !nea) then begin nea := f :: !nea; Printf.printf "%s:%d %s not enough args\n" !thefile myline f; end) else let foundid = List.nth ids n in let efoundid = List.nth (List.rev ids) n1 in if not(foundid = i || efoundid = i) then Printf.printf "%s:%d %s wrong arg in position %d: %s\n" !thefile myline i n foundid); true end else false) !tbl in ()
@script:ocaml@ @@ List.iter (function (st,fn,nm,ids) -> if List.mem nm !parsed then let entry = try !(Hashtbl.find success (st,fn,nm)) with Not_found -> [] in List.iter (fun id -> if not (List.mem id entry) && not (id = "...") then Printf.printf "%s:%d %s not used\n" !thefile st id) ids) !tbl // </smpl>
---
drivers/clk/keystone/pll.c | 4 ++-- drivers/clk/sunxi/clk-mod0.c | 2 +- drivers/clk/tegra/cvb.c | 10 +++++----- drivers/dma-buf/sw_sync.c | 6 +++--- drivers/gpu/drm/gma500/intel_i2c.c | 3 +-- drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- drivers/irqchip/irq-metag-ext.c | 1 - drivers/irqchip/irq-vic.c | 1 - drivers/mfd/tc3589x.c | 4 ++-- drivers/power/supply/ab8500_fg.c | 8 ++++---- drivers/power/supply/abx500_chargalg.c | 1 + drivers/power/supply/intel_mid_battery.c | 2 +- drivers/power/supply/power_supply_core.c | 4 ++-- fs/crypto/crypto.c | 4 ++-- fs/crypto/fname.c | 4 ++-- fs/ubifs/file.c | 2 +- fs/ubifs/gc.c | 2 +- fs/ubifs/lprops.c | 2 +- fs/ubifs/lpt_commit.c | 4 +--- fs/ubifs/replay.c | 2 +- lib/kobject_uevent.c | 6 +++--- lib/lru_cache.c | 4 ++-- lib/nlattr.c | 2 +- 23 files changed, 39 insertions(+), 43 deletions(-)
Use the names of the corresponding functions.
Issue detected using Coccinelle (http://coccinelle.lip6.fr/)
Signed-off-by: Julia Lawall Julia.Lawall@lip6.fr
--- drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index e1cfba5..8d93355 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -635,7 +635,7 @@ static int ioctl_gem_info(struct drm_device *dev, void *data, */
/** - * load - setup chip and create an initial config + * dev_load - setup chip and create an initial config * @dev: DRM device * @flags: startup flags * @@ -733,7 +733,7 @@ static int dev_open(struct drm_device *dev, struct drm_file *file) }
/** - * lastclose - clean up after all DRM clients have exited + * dev_lastclose - clean up after all DRM clients have exited * @dev: DRM device * * Take care of cleaning up after all DRM clients have exited. In the
Adjust the documentation to use the names that appear in the function parameter list.
Issue detected using Coccinelle (http://coccinelle.lip6.fr/)
Signed-off-by: Julia Lawall Julia.Lawall@lip6.fr
--- drivers/dma-buf/sw_sync.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 62e8e6d..5d2b1b6 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -155,11 +155,11 @@ static void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
/** * sync_pt_create() - creates a sync pt - * @parent: fence's parent sync_timeline + * @obj: fence's parent sync_timeline * @size: size to allocate for this pt - * @inc: value of the fence + * @value: value of the fence * - * Creates a new sync_pt as a child of @parent. @size bytes will be + * Creates a new sync_pt as a child of @obj. @size bytes will be * allocated allowing for implementation specific data to be kept after * the generic sync_timeline struct. Returns the sync_pt object or * NULL in case of error.
Delete a reference to a variable that has never been used by the function in the first case, and rename a variable to that of the corresponding parameter name in the second case.
Issue detected using Coccinelle (http://coccinelle.lip6.fr/)
Signed-off-by: Julia Lawall Julia.Lawall@lip6.fr
--- drivers/gpu/drm/gma500/intel_i2c.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/gma500/intel_i2c.c b/drivers/gpu/drm/gma500/intel_i2c.c index 98a28c2..12c40f1 100644 --- a/drivers/gpu/drm/gma500/intel_i2c.c +++ b/drivers/gpu/drm/gma500/intel_i2c.c @@ -95,7 +95,6 @@ static void set_data(void *data, int state_high) /** * psb_intel_i2c_create - instantiate an Intel i2c bus using the specified GPIO reg * @dev: DRM device - * @output: driver specific output device * @reg: GPIO reg to use * @name: name for this bus * @@ -155,7 +154,7 @@ struct psb_intel_i2c_chan *psb_intel_i2c_create(struct drm_device *dev,
/** * psb_intel_i2c_destroy - unregister and free i2c bus resources - * @output: channel to free + * @chan: channel to free * * Unregister the adapter from the i2c layer, then free the structure. */
On Sat, 2016-10-01 at 21:46 +0200, Julia Lawall wrote:
These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely.
Hi Julia.
Would it be possible for a semantic patch to scan for function definitions where the types do not have identifiers and update the definitions to match the declarations?
For instance, given:
<some.h> int foo(int);
<some.c> int foo(int bar) { return baz; }
Could coccinelle output:
diff a/some.h b/some.h [] -int foo(int); +int foo(int bar);
On Sat, 1 Oct 2016, Joe Perches wrote:
On Sat, 2016-10-01 at 21:46 +0200, Julia Lawall wrote:
These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely.
Hi Julia.
Would it be possible for a semantic patch to scan for function definitions where the types do not have identifiers and update the definitions to match the declarations?
For instance, given:
<some.h> int foo(int);
<some.c> int foo(int bar) { return baz; }
Could coccinelle output:
diff a/some.h b/some.h [] -int foo(int); +int foo(int bar);
The following seems to work:
@r@ identifier f; position p; type T, t; parameter list[n] ps; @@
T f@p(ps,t,...);
@s@ identifier r.f,x; type r.T, r.t; parameter list[r.n] ps; @@
T f(ps,t x,...) { ... }
@@ identifier r.f, s.x; position r.p; type r.T, r.t; parameter list[r.n] ps; @@
T f@p(ps,t + x ,...);
After letting it run for a few minutes without making any effort to include .h files, I get over 2700 changed lines.
julia
Jani Nikula has a patch with a scrip to make the one kernel-doc parser into a lint/checker pass over the entire kernel. I think that'd would be more robust instead of trying to approximate the real kerneldoc parser. Otoh that parser is a horror show of a perl/regex driven state machine ;-)
Jani, can you pls digg out these patches? Can't find them right now ... -Daniel
On Sat, Oct 1, 2016 at 9:46 PM, Julia Lawall Julia.Lawall@lip6.fr wrote:
These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely.
// <smpl> @initialize:ocaml@ @@
let tbl = ref [] let fnstart = ref [] let success = Hashtbl.create 101 let thefile = ref "" let parsed = ref [] let nea = ref []
let parse file = thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1; let i = open_in file in let startline = ref 0 in let fn = ref "" in let ids = ref [] in let rec inside n = let l = input_line i in let n = n + 1 in match Str.split_delim (Str.regexp_string "*/") l with before::after::_ -> (if not (!fn = "") then tbl := (!startline,n,!fn,List.rev !ids)::!tbl); startline := 0; fn := ""; ids := []; outside n | _ -> (match Str.split (Str.regexp "[ \t]+") l with "*"::name::rest -> let len = String.length name in (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()" then fn := String.sub name 0 (len-2) else if !fn = "" && (not (rest = [])) && List.hd rest = "-" then if String.get name (len-1) = ':' then fn := String.sub name 0 (len-1) else fn := name else if not(!fn = "") && len > 2 && String.get name 0 = '@' && String.get name (len-1) = ':' then ids := (String.sub name 1 (len-2)) :: !ids); | _ -> ()); inside n and outside n = let l = input_line i in let n = n + 1 in if String.length l > 2 && String.sub l 0 3 = "/**" then begin startline := n; inside n end else outside n in try outside 0 with End_of_file -> ()
let hashadd tbl k v = let cell = try Hashtbl.find tbl k with Not_found -> let cell = ref [] in Hashtbl.add tbl k cell; cell in cell := v :: !cell
@script:ocaml@ @@
tbl := []; fnstart := []; Hashtbl.clear success; parsed := []; nea := []; parse (List.hd (Coccilib.files()))
@r@ identifier f; position p; @@
f@p(...) { ... }
@script:ocaml@ p << r.p; f << r.f; @@
parsed := f :: !parsed; fnstart := (List.hd p).line :: !fnstart
@param@ identifier f; type T; identifier i; parameter list[n] ps; parameter list[n1] ps1; position p; @@
f@p(ps,T i,ps1) { ... }
@script:ocaml@ @@
tbl := List.rev (List.sort compare !tbl)
@script:ocaml@ p << param.p; f << param.f; @@
let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if not (String.lowercase f = String.lowercase nm) then Printf.printf "%s:%d %s doesn't match preceding comment: %s\n" !thefile myline f nm); true end else false) !tbl in ()
@script:ocaml@ p << param.p; n << param.n; n1 << param.n1; i << param.i; f << param.f; @@
let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if List.mem i ids then hashadd success (st,fn,nm) i); (if ids = [] (* arg list seems not obligatory *) then () else if not (List.mem i ids) then Printf.printf "%s:%d %s doesn't appear in ids: %s\n" !thefile myline i (String.concat " " ids) else if List.length ids <= n || List.length ids <= n1 then (if not (List.mem f !nea) then begin nea := f :: !nea; Printf.printf "%s:%d %s not enough args\n" !thefile myline f; end) else let foundid = List.nth ids n in let efoundid = List.nth (List.rev ids) n1 in if not(foundid = i || efoundid = i) then Printf.printf "%s:%d %s wrong arg in position %d: %s\n" !thefile myline i n foundid); true end else false) !tbl in ()
@script:ocaml@ @@ List.iter (function (st,fn,nm,ids) -> if List.mem nm !parsed then let entry = try !(Hashtbl.find success (st,fn,nm)) with Not_found -> [] in List.iter (fun id -> if not (List.mem id entry) && not (id = "...") then Printf.printf "%s:%d %s not used\n" !thefile st id) ids) !tbl // </smpl>
drivers/clk/keystone/pll.c | 4 ++-- drivers/clk/sunxi/clk-mod0.c | 2 +- drivers/clk/tegra/cvb.c | 10 +++++----- drivers/dma-buf/sw_sync.c | 6 +++--- drivers/gpu/drm/gma500/intel_i2c.c | 3 +-- drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- drivers/irqchip/irq-metag-ext.c | 1 - drivers/irqchip/irq-vic.c | 1 - drivers/mfd/tc3589x.c | 4 ++-- drivers/power/supply/ab8500_fg.c | 8 ++++---- drivers/power/supply/abx500_chargalg.c | 1 + drivers/power/supply/intel_mid_battery.c | 2 +- drivers/power/supply/power_supply_core.c | 4 ++-- fs/crypto/crypto.c | 4 ++-- fs/crypto/fname.c | 4 ++-- fs/ubifs/file.c | 2 +- fs/ubifs/gc.c | 2 +- fs/ubifs/lprops.c | 2 +- fs/ubifs/lpt_commit.c | 4 +--- fs/ubifs/replay.c | 2 +- lib/kobject_uevent.c | 6 +++--- lib/lru_cache.c | 4 ++-- lib/nlattr.c | 2 +- 23 files changed, 39 insertions(+), 43 deletions(-) _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, 05 Oct 2016, Daniel Vetter daniel@ffwll.ch wrote:
Jani Nikula has a patch with a scrip to make the one kernel-doc parser into a lint/checker pass over the entire kernel. I think that'd would be more robust instead of trying to approximate the real kerneldoc parser. Otoh that parser is a horror show of a perl/regex driven state machine ;-)
Jani, can you pls digg out these patches? Can't find them right now ...
Expanding the massive Cc: with linux-doc list...
Here goes. It's a quick hack from months ago, but still seems to somewhat work. At least for the kernel-doc parts. The reStructuredText lint part isn't all that great, and doesn't have mapping to line numbers like the Sphinx kernel-doc extension does. Anyway I'm happy how this integrates with kernel build CHECK and C=1/C=2.
I guess Julia's goal is to automate the *fixing* of some of the error classes from kernel-doc. Not sure how well this could be made to integrate with any of that.
BR, Jani.
From 1244efa0f63a7b13795e8c37f81733a3c8bfc56a Mon Sep 17 00:00:00 2001
From: Jani Nikula jani.nikula@intel.com Date: Tue, 31 May 2016 18:11:33 +0300 Subject: [PATCH] kernel-doc-rst-lint: add tool to check kernel-doc and rst correctness Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Cc: Jani Nikula jani.nikula@intel.com
Simple kernel-doc and reStructuredText lint tool that can be used independently and as a kernel build CHECK tool to validate kernel-doc comments.
Independent usage: $ kernel-doc-rst-lint FILE
Kernel CHECK usage: $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2)
Depends on docutils and the rst-lint package https://pypi.python.org/pypi/restructuredtext_lint
Signed-off-by: Jani Nikula jani.nikula@intel.com --- scripts/kernel-doc-rst-lint | 106 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100755 scripts/kernel-doc-rst-lint
diff --git a/scripts/kernel-doc-rst-lint b/scripts/kernel-doc-rst-lint new file mode 100755 index 000000000000..7e0157679f83 --- /dev/null +++ b/scripts/kernel-doc-rst-lint @@ -0,0 +1,106 @@ +#!/usr/bin/env python +# coding=utf-8 +# +# Copyright © 2016 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# +# Authors: +# Jani Nikula jani.nikula@intel.com +# +# Simple kernel-doc and reStructuredText lint tool that can be used +# independently and as a kernel build CHECK tool to validate kernel-doc +# comments. +# +# Independent usage: +# $ kernel-doc-rst-lint FILE +# +# Kernel CHECK usage: +# $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2) +# +# Depends on docutils and the rst-lint package +# https://pypi.python.org/pypi/restructuredtext_lint +# + +import os +import subprocess +import sys + +from docutils.parsers.rst import directives +from docutils.parsers.rst import Directive +from docutils.parsers.rst import roles +from docutils import nodes, statemachine +import restructuredtext_lint + +class DummyDirective(Directive): + required_argument = 1 + optional_arguments = 0 + option_spec = { } + has_content = True + + def run(self): + return [] + +# Fake the Sphinx C Domain directives and roles +directives.register_directive('c:function', DummyDirective) +directives.register_directive('c:type', DummyDirective) +roles.register_generic_role('c:func', nodes.emphasis) +roles.register_generic_role('c:type', nodes.emphasis) + +# We accept but ignore parameters to be compatible with how the kernel build +# invokes CHECK. +if len(sys.argv) < 2: + sys.stderr.write('usage: kernel-doc-rst-lint [IGNORED OPTIONS] FILE\n'); + sys.exit(1) + +infile = sys.argv[len(sys.argv) - 1] +cmd = ['scripts/kernel-doc', '-rst', infile] + +try: + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True) + out, err = p.communicate() + + # python2 needs conversion to unicode. + # python3 with universal_newlines=True returns strings. + if sys.version_info.major < 3: + out, err = unicode(out, 'utf-8'), unicode(err, 'utf-8') + + # kernel-doc errors + sys.stderr.write(err) + if p.returncode != 0: + sys.exit(p.returncode) + + # restructured text errors + lines = statemachine.string2lines(out, 8, convert_whitespace=True) + lint_errors = restructuredtext_lint.lint(out, infile) + for error in lint_errors: + # Ignore INFO + if error.level <= 1: + continue + + print(error.source + ': ' + error.type + ': ' + error.full_message) + if error.line is not None: + print('Context:') + print('\t' + lines[error.line - 1]) + print('\t' + lines[error.line]) + +except Exception as e: + sys.stderr.write(str(e) + '\n') + sys.exit(1)
Am 05.10.2016 um 16:04 schrieb Jani Nikula jani.nikula@linux.intel.com:
On Wed, 05 Oct 2016, Daniel Vetter daniel@ffwll.ch wrote:
Jani Nikula has a patch with a scrip to make the one kernel-doc parser into a lint/checker pass over the entire kernel. I think that'd would be more robust instead of trying to approximate the real kerneldoc parser. Otoh that parser is a horror show of a perl/regex driven state machine ;-)
Jani, can you pls digg out these patches? Can't find them right now ...
Expanding the massive Cc: with linux-doc list...
Here goes. It's a quick hack from months ago, but still seems to somewhat work. At least for the kernel-doc parts. The reStructuredText lint part isn't all that great, and doesn't have mapping to line numbers like the Sphinx kernel-doc extension does. Anyway I'm happy how this integrates with kernel build CHECK and C=1/C=2.
I guess Julia's goal is to automate the *fixing* of some of the error classes from kernel-doc. Not sure how well this could be made to integrate with any of that.
BR, Jani.
Another lint alternative:
use the lint from the linuxdoc project
install the linuxdoc package:
* https://return42.github.io/linuxdoc/install.html
e.g.::
pip install --user git+http://github.com/return42/linuxdoc.git
and run kernel-lintdoc with the file/folder to lint as argument / e.g.::
kernel-lintdoc include/media/
-- Markus --
From 1244efa0f63a7b13795e8c37f81733a3c8bfc56a Mon Sep 17 00:00:00 2001 From: Jani Nikula jani.nikula@intel.com Date: Tue, 31 May 2016 18:11:33 +0300 Subject: [PATCH] kernel-doc-rst-lint: add tool to check kernel-doc and rst correctness Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Cc: Jani Nikula jani.nikula@intel.com
Simple kernel-doc and reStructuredText lint tool that can be used independently and as a kernel build CHECK tool to validate kernel-doc comments.
Independent usage: $ kernel-doc-rst-lint FILE
Kernel CHECK usage: $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2)
Depends on docutils and the rst-lint package https://pypi.python.org/pypi/restructuredtext_lint
Signed-off-by: Jani Nikula jani.nikula@intel.com
scripts/kernel-doc-rst-lint | 106 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100755 scripts/kernel-doc-rst-lint
diff --git a/scripts/kernel-doc-rst-lint b/scripts/kernel-doc-rst-lint new file mode 100755 index 000000000000..7e0157679f83 --- /dev/null +++ b/scripts/kernel-doc-rst-lint @@ -0,0 +1,106 @@ +#!/usr/bin/env python +# coding=utf-8 +# +# Copyright © 2016 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# +# Authors: +# Jani Nikula jani.nikula@intel.com +# +# Simple kernel-doc and reStructuredText lint tool that can be used +# independently and as a kernel build CHECK tool to validate kernel-doc +# comments. +# +# Independent usage: +# $ kernel-doc-rst-lint FILE +# +# Kernel CHECK usage: +# $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2) +# +# Depends on docutils and the rst-lint package +# https://pypi.python.org/pypi/restructuredtext_lint +#
+import os +import subprocess +import sys
+from docutils.parsers.rst import directives +from docutils.parsers.rst import Directive +from docutils.parsers.rst import roles +from docutils import nodes, statemachine +import restructuredtext_lint
+class DummyDirective(Directive):
- required_argument = 1
- optional_arguments = 0
- option_spec = { }
- has_content = True
- def run(self):
return []+# Fake the Sphinx C Domain directives and roles +directives.register_directive('c:function', DummyDirective) +directives.register_directive('c:type', DummyDirective) +roles.register_generic_role('c:func', nodes.emphasis) +roles.register_generic_role('c:type', nodes.emphasis)
+# We accept but ignore parameters to be compatible with how the kernel build +# invokes CHECK. +if len(sys.argv) < 2:
- sys.stderr.write('usage: kernel-doc-rst-lint [IGNORED OPTIONS] FILE\n');
- sys.exit(1)
+infile = sys.argv[len(sys.argv) - 1] +cmd = ['scripts/kernel-doc', '-rst', infile]
+try:
- p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
- out, err = p.communicate()
- # python2 needs conversion to unicode.
- # python3 with universal_newlines=True returns strings.
- if sys.version_info.major < 3:
out, err = unicode(out, 'utf-8'), unicode(err, 'utf-8')- # kernel-doc errors
- sys.stderr.write(err)
- if p.returncode != 0:
sys.exit(p.returncode)- # restructured text errors
- lines = statemachine.string2lines(out, 8, convert_whitespace=True)
- lint_errors = restructuredtext_lint.lint(out, infile)
- for error in lint_errors:
# Ignore INFOif error.level <= 1:continueprint(error.source + ': ' + error.type + ': ' + error.full_message)if error.line is not None:print('Context:')print('\t' + lines[error.line - 1])print('\t' + lines[error.line])+except Exception as e:
- sys.stderr.write(str(e) + '\n')
- sys.exit(1)
-- 2.1.4
-- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 5 Oct 2016, Jani Nikula wrote:
On Wed, 05 Oct 2016, Daniel Vetter daniel@ffwll.ch wrote:
Jani Nikula has a patch with a scrip to make the one kernel-doc parser into a lint/checker pass over the entire kernel. I think that'd would be more robust instead of trying to approximate the real kerneldoc parser. Otoh that parser is a horror show of a perl/regex driven state machine ;-)
Jani, can you pls digg out these patches? Can't find them right now ...
Expanding the massive Cc: with linux-doc list...
Here goes. It's a quick hack from months ago, but still seems to somewhat work. At least for the kernel-doc parts. The reStructuredText lint part isn't all that great, and doesn't have mapping to line numbers like the Sphinx kernel-doc extension does. Anyway I'm happy how this integrates with kernel build CHECK and C=1/C=2.
I guess Julia's goal is to automate the *fixing* of some of the error classes from kernel-doc. Not sure how well this could be made to integrate with any of that.
No, my work doesn't fix anything. Coccinelle can't actually process comments. I just correlated the parsed comment with the function header.
julia
BR, Jani.
From 1244efa0f63a7b13795e8c37f81733a3c8bfc56a Mon Sep 17 00:00:00 2001 From: Jani Nikula jani.nikula@intel.com Date: Tue, 31 May 2016 18:11:33 +0300 Subject: [PATCH] kernel-doc-rst-lint: add tool to check kernel-doc and rst correctness Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo Cc: Jani Nikula jani.nikula@intel.com
Simple kernel-doc and reStructuredText lint tool that can be used independently and as a kernel build CHECK tool to validate kernel-doc comments.
Independent usage: $ kernel-doc-rst-lint FILE
Kernel CHECK usage: $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2)
Depends on docutils and the rst-lint package https://pypi.python.org/pypi/restructuredtext_lint
Signed-off-by: Jani Nikula jani.nikula@intel.com
scripts/kernel-doc-rst-lint | 106 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100755 scripts/kernel-doc-rst-lint
diff --git a/scripts/kernel-doc-rst-lint b/scripts/kernel-doc-rst-lint new file mode 100755 index 000000000000..7e0157679f83 --- /dev/null +++ b/scripts/kernel-doc-rst-lint @@ -0,0 +1,106 @@ +#!/usr/bin/env python +# coding=utf-8 +# +# Copyright © 2016 Intel Corporation +# +# Permission is hereby granted, free of charge, to any person obtaining a +# copy of this software and associated documentation files (the "Software"), +# to deal in the Software without restriction, including without limitation +# the rights to use, copy, modify, merge, publish, distribute, sublicense, +# and/or sell copies of the Software, and to permit persons to whom the +# Software is furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice (including the next +# paragraph) shall be included in all copies or substantial portions of the +# Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +# IN THE SOFTWARE. +# +# Authors: +# Jani Nikula jani.nikula@intel.com +# +# Simple kernel-doc and reStructuredText lint tool that can be used +# independently and as a kernel build CHECK tool to validate kernel-doc +# comments. +# +# Independent usage: +# $ kernel-doc-rst-lint FILE +# +# Kernel CHECK usage: +# $ make CHECK=scripts/kernel-doc-rst-lint C=1 # (or C=2) +# +# Depends on docutils and the rst-lint package +# https://pypi.python.org/pypi/restructuredtext_lint +#
+import os +import subprocess +import sys
+from docutils.parsers.rst import directives +from docutils.parsers.rst import Directive +from docutils.parsers.rst import roles +from docutils import nodes, statemachine +import restructuredtext_lint
+class DummyDirective(Directive):
- required_argument = 1
- optional_arguments = 0
- option_spec = { }
- has_content = True
- def run(self):
return []+# Fake the Sphinx C Domain directives and roles +directives.register_directive('c:function', DummyDirective) +directives.register_directive('c:type', DummyDirective) +roles.register_generic_role('c:func', nodes.emphasis) +roles.register_generic_role('c:type', nodes.emphasis)
+# We accept but ignore parameters to be compatible with how the kernel build +# invokes CHECK. +if len(sys.argv) < 2:
- sys.stderr.write('usage: kernel-doc-rst-lint [IGNORED OPTIONS] FILE\n');
- sys.exit(1)
+infile = sys.argv[len(sys.argv) - 1] +cmd = ['scripts/kernel-doc', '-rst', infile]
+try:
- p = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
- out, err = p.communicate()
- # python2 needs conversion to unicode.
- # python3 with universal_newlines=True returns strings.
- if sys.version_info.major < 3:
out, err = unicode(out, 'utf-8'), unicode(err, 'utf-8')- # kernel-doc errors
- sys.stderr.write(err)
- if p.returncode != 0:
sys.exit(p.returncode)- # restructured text errors
- lines = statemachine.string2lines(out, 8, convert_whitespace=True)
- lint_errors = restructuredtext_lint.lint(out, infile)
- for error in lint_errors:
# Ignore INFOif error.level <= 1:continueprint(error.source + ': ' + error.type + ': ' + error.full_message)if error.line is not None:print('Context:')print('\t' + lines[error.line - 1])print('\t' + lines[error.line])+except Exception as e:
- sys.stderr.write(str(e) + '\n')
- sys.exit(1)
-- 2.1.4
-- Jani Nikula, Intel Open Source Technology Center -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 5 Oct 2016, Daniel Vetter wrote:
Jani Nikula has a patch with a scrip to make the one kernel-doc parser into a lint/checker pass over the entire kernel. I think that'd would be more robust instead of trying to approximate the real kerneldoc parser. Otoh that parser is a horror show of a perl/regex driven state machine ;-)
Sure. To my recollection, I found around 2000 issues. Many I ignored, eg functions that simply have no documentation abuot the parameters, functions that document their local variables, when these were more interesting than the parameters etc. But the set of patches is not exhaustive with respect to the remaining interesting ones either.
julia
Jani, can you pls digg out these patches? Can't find them right now ... -Daniel
On Sat, Oct 1, 2016 at 9:46 PM, Julia Lawall Julia.Lawall@lip6.fr wrote:
These patches fix cases where the documentation above a function definition is not consistent with the function header. Issues are detected using the semantic patch below (http://coccinelle.lip6.fr/). Basically, the semantic patch parses a file to find comments, then matches each function header, and checks that the name and parameter list in the function header are compatible with the comment that preceeds it most closely.
// <smpl> @initialize:ocaml@ @@
let tbl = ref [] let fnstart = ref [] let success = Hashtbl.create 101 let thefile = ref "" let parsed = ref [] let nea = ref []
let parse file = thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1; let i = open_in file in let startline = ref 0 in let fn = ref "" in let ids = ref [] in let rec inside n = let l = input_line i in let n = n + 1 in match Str.split_delim (Str.regexp_string "*/") l with before::after::_ -> (if not (!fn = "") then tbl := (!startline,n,!fn,List.rev !ids)::!tbl); startline := 0; fn := ""; ids := []; outside n | _ -> (match Str.split (Str.regexp "[ \t]+") l with "*"::name::rest -> let len = String.length name in (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()" then fn := String.sub name 0 (len-2) else if !fn = "" && (not (rest = [])) && List.hd rest = "-" then if String.get name (len-1) = ':' then fn := String.sub name 0 (len-1) else fn := name else if not(!fn = "") && len > 2 && String.get name 0 = '@' && String.get name (len-1) = ':' then ids := (String.sub name 1 (len-2)) :: !ids); | _ -> ()); inside n and outside n = let l = input_line i in let n = n + 1 in if String.length l > 2 && String.sub l 0 3 = "/**" then begin startline := n; inside n end else outside n in try outside 0 with End_of_file -> ()
let hashadd tbl k v = let cell = try Hashtbl.find tbl k with Not_found -> let cell = ref [] in Hashtbl.add tbl k cell; cell in cell := v :: !cell
@script:ocaml@ @@
tbl := []; fnstart := []; Hashtbl.clear success; parsed := []; nea := []; parse (List.hd (Coccilib.files()))
@r@ identifier f; position p; @@
f@p(...) { ... }
@script:ocaml@ p << r.p; f << r.f; @@
parsed := f :: !parsed; fnstart := (List.hd p).line :: !fnstart
@param@ identifier f; type T; identifier i; parameter list[n] ps; parameter list[n1] ps1; position p; @@
f@p(ps,T i,ps1) { ... }
@script:ocaml@ @@
tbl := List.rev (List.sort compare !tbl)
@script:ocaml@ p << param.p; f << param.f; @@
let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if not (String.lowercase f = String.lowercase nm) then Printf.printf "%s:%d %s doesn't match preceding comment: %s\n" !thefile myline f nm); true end else false) !tbl in ()
@script:ocaml@ p << param.p; n << param.n; n1 << param.n1; i << param.i; f << param.f; @@
let myline = (List.hd p).line in let prevline = List.fold_left (fun prev x -> if x < myline then max x prev else prev) 0 !fnstart in let _ = List.exists (function (st,fn,nm,ids) -> if prevline < st && myline > st && prevline < fn && myline > fn then begin (if List.mem i ids then hashadd success (st,fn,nm) i); (if ids = [] (* arg list seems not obligatory *) then () else if not (List.mem i ids) then Printf.printf "%s:%d %s doesn't appear in ids: %s\n" !thefile myline i (String.concat " " ids) else if List.length ids <= n || List.length ids <= n1 then (if not (List.mem f !nea) then begin nea := f :: !nea; Printf.printf "%s:%d %s not enough args\n" !thefile myline f; end) else let foundid = List.nth ids n in let efoundid = List.nth (List.rev ids) n1 in if not(foundid = i || efoundid = i) then Printf.printf "%s:%d %s wrong arg in position %d: %s\n" !thefile myline i n foundid); true end else false) !tbl in ()
@script:ocaml@ @@ List.iter (function (st,fn,nm,ids) -> if List.mem nm !parsed then let entry = try !(Hashtbl.find success (st,fn,nm)) with Not_found -> [] in List.iter (fun id -> if not (List.mem id entry) && not (id = "...") then Printf.printf "%s:%d %s not used\n" !thefile st id) ids) !tbl // </smpl>
drivers/clk/keystone/pll.c | 4 ++-- drivers/clk/sunxi/clk-mod0.c | 2 +- drivers/clk/tegra/cvb.c | 10 +++++----- drivers/dma-buf/sw_sync.c | 6 +++--- drivers/gpu/drm/gma500/intel_i2c.c | 3 +-- drivers/gpu/drm/omapdrm/omap_drv.c | 4 ++-- drivers/irqchip/irq-metag-ext.c | 1 - drivers/irqchip/irq-vic.c | 1 - drivers/mfd/tc3589x.c | 4 ++-- drivers/power/supply/ab8500_fg.c | 8 ++++---- drivers/power/supply/abx500_chargalg.c | 1 + drivers/power/supply/intel_mid_battery.c | 2 +- drivers/power/supply/power_supply_core.c | 4 ++-- fs/crypto/crypto.c | 4 ++-- fs/crypto/fname.c | 4 ++-- fs/ubifs/file.c | 2 +- fs/ubifs/gc.c | 2 +- fs/ubifs/lprops.c | 2 +- fs/ubifs/lpt_commit.c | 4 +--- fs/ubifs/replay.c | 2 +- lib/kobject_uevent.c | 6 +++--- lib/lru_cache.c | 4 ++-- lib/nlattr.c | 2 +- 23 files changed, 39 insertions(+), 43 deletions(-) _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Jani Nikula -
Joe Perches -
Julia Lawall -
Julia Lawall
-
Markus Heiser