* [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup()
@ 2017-12-01 10:47 Sven Eckelmann
2017-12-01 10:47 ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions Sven Eckelmann
2017-12-01 11:06 ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup() Simon Wunderlich
0 siblings, 2 replies; 5+ messages in thread
From: Sven Eckelmann @ 2017-12-01 10:47 UTC (permalink / raw)
To: b.a.t.m.a.n
From: Kees Cook <keescook@chromium.org>
This converts all remaining cases of the old setup_timer() API into using
timer_setup(), where the callback argument is the structure already
holding the struct timer_list. These should have no behavioral changes,
since they just change which pointer is passed into the callback with
the same available pointers after conversion.
Signed-off-by: Kees Cook <keescook@chromium.org>
[sven@narfation.org: removed spatch and examples, added compat code]
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
compat-include/linux/timer.h | 59 ++++++++++++++++++++++++++++++++++++++++++++
net/batman-adv/tp_meter.c | 14 +++++------
2 files changed, 65 insertions(+), 8 deletions(-)
create mode 100644 compat-include/linux/timer.h
diff --git a/compat-include/linux/timer.h b/compat-include/linux/timer.h
new file mode 100644
index 0000000..750b6ae
--- /dev/null
+++ b/compat-include/linux/timer.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2007-2017 B.A.T.M.A.N. contributors:
+ *
+ * Marek Lindner, Simon Wunderlich
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ *
+ * License-Filename: LICENSES/preferred/GPL-2.0
+ *
+ * This file contains macros for maintaining compatibility with older versions
+ * of the Linux kernel.
+ */
+
+#ifndef _NET_BATMAN_ADV_COMPAT_LINUX_TIMER_H
+#define _NET_BATMAN_ADV_COMPAT_LINUX_TIMER_H
+
+#include <linux/version.h>
+#include_next <linux/timer.h>
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3, 7, 0)
+
+#define __setup_timer(_timer, _fn, _data, _flags) \
+ do { \
+ init_timer(_timer); \
+ (_timer)->function = (_fn); \
+ (_timer)->data = (_data); \
+ } while (0)
+
+#endif /* < KERNEL_VERSION(3, 7, 0) */
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 14, 0)
+
+#define TIMER_DATA_TYPE unsigned long
+#define TIMER_FUNC_TYPE void (*)(TIMER_DATA_TYPE)
+
+static inline void timer_setup(struct timer_list *timer,
+ void (*callback)(struct timer_list *),
+ unsigned int flags)
+{
+ __setup_timer(timer, (TIMER_FUNC_TYPE)callback,
+ (TIMER_DATA_TYPE)timer, flags);
+}
+
+#define from_timer(var, callback_timer, timer_fieldname) \
+ container_of(callback_timer, typeof(*var), timer_fieldname)
+
+#endif /* < KERNEL_VERSION(4, 14, 0) */
+
+#endif /* _NET_BATMAN_ADV_COMPAT_LINUX_TIMER_H */
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 4b90033..15cd213 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -488,9 +488,9 @@ static void batadv_tp_reset_sender_timer(struct batadv_tp_vars *tp_vars)
* Switch to Slow Start, set the ss_threshold to half of the current cwnd and
* reset the cwnd to 3*MSS
*/
-static void batadv_tp_sender_timeout(unsigned long arg)
+static void batadv_tp_sender_timeout(struct timer_list *t)
{
- struct batadv_tp_vars *tp_vars = (struct batadv_tp_vars *)arg;
+ struct batadv_tp_vars *tp_vars = from_timer(tp_vars, t, timer);
struct batadv_priv *bat_priv = tp_vars->bat_priv;
if (atomic_read(&tp_vars->sending) == 0)
@@ -1020,8 +1020,7 @@ void batadv_tp_start(struct batadv_priv *bat_priv, const u8 *dst,
atomic64_set(&tp_vars->tot_sent, 0);
kref_get(&tp_vars->refcount);
- setup_timer(&tp_vars->timer, batadv_tp_sender_timeout,
- (unsigned long)tp_vars);
+ timer_setup(&tp_vars->timer, batadv_tp_sender_timeout, 0);
tp_vars->bat_priv = bat_priv;
tp_vars->start_time = jiffies;
@@ -1109,9 +1108,9 @@ static void batadv_tp_reset_receiver_timer(struct batadv_tp_vars *tp_vars)
* reached without received ack
* @arg: address of the related tp_vars
*/
-static void batadv_tp_receiver_shutdown(unsigned long arg)
+static void batadv_tp_receiver_shutdown(struct timer_list *t)
{
- struct batadv_tp_vars *tp_vars = (struct batadv_tp_vars *)arg;
+ struct batadv_tp_vars *tp_vars = from_timer(tp_vars, t, timer);
struct batadv_tp_unacked *un, *safe;
struct batadv_priv *bat_priv;
@@ -1373,8 +1372,7 @@ batadv_tp_init_recv(struct batadv_priv *bat_priv,
hlist_add_head_rcu(&tp_vars->list, &bat_priv->tp_list);
kref_get(&tp_vars->refcount);
- setup_timer(&tp_vars->timer, batadv_tp_receiver_shutdown,
- (unsigned long)tp_vars);
+ timer_setup(&tp_vars->timer, batadv_tp_receiver_shutdown, 0);
batadv_tp_reset_receiver_timer(tp_vars);
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions
2017-12-01 10:47 [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup() Sven Eckelmann
@ 2017-12-01 10:47 ` Sven Eckelmann
2017-12-01 16:40 ` Kees Cook
2017-12-01 11:06 ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup() Simon Wunderlich
1 sibling, 1 reply; 5+ messages in thread
From: Sven Eckelmann @ 2017-12-01 10:47 UTC (permalink / raw)
To: b.a.t.m.a.n; +Cc: Sven Eckelmann, Kees Cook
The commit e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
changed the argument name and type of the timer function but didn't adjust
the kernel-doc of these functions.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
Cc: Kees Cook <keescook@chromium.org>
---
net/batman-adv/tp_meter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
index 15cd213..ebc4e22 100644
--- a/net/batman-adv/tp_meter.c
+++ b/net/batman-adv/tp_meter.c
@@ -482,7 +482,7 @@ static void batadv_tp_reset_sender_timer(struct batadv_tp_vars *tp_vars)
/**
* batadv_tp_sender_timeout - timer that fires in case of packet loss
- * @arg: address of the related tp_vars
+ * @t: address to timer_list inside tp_vars
*
* If fired it means that there was packet loss.
* Switch to Slow Start, set the ss_threshold to half of the current cwnd and
@@ -1106,7 +1106,7 @@ static void batadv_tp_reset_receiver_timer(struct batadv_tp_vars *tp_vars)
/**
* batadv_tp_receiver_shutdown - stop a tp meter receiver when timeout is
* reached without received ack
- * @arg: address of the related tp_vars
+ * @t: address to timer_list inside tp_vars
*/
static void batadv_tp_receiver_shutdown(struct timer_list *t)
{
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup()
2017-12-01 10:47 [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup() Sven Eckelmann
2017-12-01 10:47 ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions Sven Eckelmann
@ 2017-12-01 11:06 ` Simon Wunderlich
1 sibling, 0 replies; 5+ messages in thread
From: Simon Wunderlich @ 2017-12-01 11:06 UTC (permalink / raw)
To: b.a.t.m.a.n
[-- Attachment #1: Type: text/plain, Size: 701 bytes --]
On Friday, December 1, 2017 11:47:55 AM CET Sven Eckelmann wrote:
> From: Kees Cook <keescook@chromium.org>
>
> This converts all remaining cases of the old setup_timer() API into using
> timer_setup(), where the callback argument is the structure already
> holding the struct timer_list. These should have no behavioral changes,
> since they just change which pointer is passed into the callback with
> the same available pointers after conversion.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> [sven@narfation.org: removed spatch and examples, added compat code]
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
Picked this series up into 295ef829 and 258e419d
Thank you!
Simon
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions
2017-12-01 10:47 ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions Sven Eckelmann
@ 2017-12-01 16:40 ` Kees Cook
2017-12-01 17:01 ` Julia Lawall
0 siblings, 1 reply; 5+ messages in thread
From: Kees Cook @ 2017-12-01 16:40 UTC (permalink / raw)
To: Thomas Gleixner, Julia Lawall; +Cc: b.a.t.m.a.n, Sven Eckelmann
On Fri, Dec 1, 2017 at 2:47 AM, Sven Eckelmann <sven@narfation.org> wrote:
> The commit e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
> changed the argument name and type of the timer function but didn't adjust
> the kernel-doc of these functions.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> Cc: Kees Cook <keescook@chromium.org>
Acked-by: Kees Cook <keescook@chromium.org>
I wonder if there is a way for Coccinelle to change kernel-doc?
-Kees
> ---
> net/batman-adv/tp_meter.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
> index 15cd213..ebc4e22 100644
> --- a/net/batman-adv/tp_meter.c
> +++ b/net/batman-adv/tp_meter.c
> @@ -482,7 +482,7 @@ static void batadv_tp_reset_sender_timer(struct batadv_tp_vars *tp_vars)
>
> /**
> * batadv_tp_sender_timeout - timer that fires in case of packet loss
> - * @arg: address of the related tp_vars
> + * @t: address to timer_list inside tp_vars
> *
> * If fired it means that there was packet loss.
> * Switch to Slow Start, set the ss_threshold to half of the current cwnd and
> @@ -1106,7 +1106,7 @@ static void batadv_tp_reset_receiver_timer(struct batadv_tp_vars *tp_vars)
> /**
> * batadv_tp_receiver_shutdown - stop a tp meter receiver when timeout is
> * reached without received ack
> - * @arg: address of the related tp_vars
> + * @t: address to timer_list inside tp_vars
> */
> static void batadv_tp_receiver_shutdown(struct timer_list *t)
> {
> --
> 2.11.0
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions
2017-12-01 16:40 ` Kees Cook
@ 2017-12-01 17:01 ` Julia Lawall
0 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2017-12-01 17:01 UTC (permalink / raw)
To: Kees Cook; +Cc: Thomas Gleixner, b.a.t.m.a.n, Sven Eckelmann
[-- Attachment #1: Type: text/plain, Size: 3173 bytes --]
On Fri, 1 Dec 2017, Kees Cook wrote:
> On Fri, Dec 1, 2017 at 2:47 AM, Sven Eckelmann <sven@narfation.org> wrote:
> > The commit e99e88a9d2b0 ("treewide: setup_timer() -> timer_setup()")
> > changed the argument name and type of the timer function but didn't adjust
> > the kernel-doc of these functions.
> >
> > Signed-off-by: Sven Eckelmann <sven@narfation.org>
> > ---
> > Cc: Kees Cook <keescook@chromium.org>
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> I wonder if there is a way for Coccinelle to change kernel-doc?
It can't change it, but with some cleverness (ie python/ocaml code) it can
be used to find problems. I've attached a semantic patch that I wrote for
this. It gives reports like:
drivers/acpi/arm64/iort.c:864 dma_size doesn't appear in ids: dev dma_addr size
Indeed the code has:
/**
* iort_dma_setup() - Set-up device DMA parameters.
*
* @dev: device to configure
* @dma_addr: device DMA address result pointer
* @size: DMA range size result pointer
*/
void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
So the kerneldoc has the wrong name. There are also things like this:
drivers/acpi/acpica/utdebug.c:617 acpi_trace_point doesn't match preceding
comment: begin
Here the code is:
/******************************************************************************\
*
*
* FUNCTION: acpi_trace_point
*
* PARAMETERS: type - Trace event type
* begin - TRUE if before execution
* aml - Executed AML address
* pathname - Object path
* pointer - Pointer to the related object
*
* RETURN: None
*
* DESCRIPTION: Interpreter execution trace.
*
******************************************************************************\
/
void
acpi_trace_point(acpi_trace_event_type type, u8 begin, u8 *aml, char *pathname)
So the rule doesn't seem to know about this kind of documentation.
julia
>
> -Kees
>
> > ---
> > net/batman-adv/tp_meter.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/batman-adv/tp_meter.c b/net/batman-adv/tp_meter.c
> > index 15cd213..ebc4e22 100644
> > --- a/net/batman-adv/tp_meter.c
> > +++ b/net/batman-adv/tp_meter.c
> > @@ -482,7 +482,7 @@ static void batadv_tp_reset_sender_timer(struct batadv_tp_vars *tp_vars)
> >
> > /**
> > * batadv_tp_sender_timeout - timer that fires in case of packet loss
> > - * @arg: address of the related tp_vars
> > + * @t: address to timer_list inside tp_vars
> > *
> > * If fired it means that there was packet loss.
> > * Switch to Slow Start, set the ss_threshold to half of the current cwnd and
> > @@ -1106,7 +1106,7 @@ static void batadv_tp_reset_receiver_timer(struct batadv_tp_vars *tp_vars)
> > /**
> > * batadv_tp_receiver_shutdown - stop a tp meter receiver when timeout is
> > * reached without received ack
> > - * @arg: address of the related tp_vars
> > + * @t: address to timer_list inside tp_vars
> > */
> > static void batadv_tp_receiver_shutdown(struct timer_list *t)
> > {
> > --
> > 2.11.0
> >
>
>
>
> --
> Kees Cook
> Pixel Security
>
[-- Attachment #2: Type: text/plain, Size: 4348 bytes --]
@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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-01 17:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-01 10:47 [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup() Sven Eckelmann
2017-12-01 10:47 ` [B.A.T.M.A.N.] [PATCH 2/2] batman-adv: Fix kernel-doc for timer functions Sven Eckelmann
2017-12-01 16:40 ` Kees Cook
2017-12-01 17:01 ` Julia Lawall
2017-12-01 11:06 ` [B.A.T.M.A.N.] [PATCH 1/2] batman-adv: setup_timer() -> timer_setup() Simon Wunderlich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.