Re: [PATCH net-next,v4 01/12] flow_offload: add flow_rule and flow_match structures and use them (fwd)

[Julia Lawall]

Hello,

It looks like the kfree on line 1573 should be moved down a few lines.

julia

-- Forwarded message --
Date: Sat, 1 Dec 2018 11:11:55 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH net-next,
v4 01/12] flow_offload: add flow_rule and flow_match structures and use them

CC: kbuild-...@01.org
In-Reply-To: <20181129022231.2740-2-pa...@netfilter.org>
References: <20181129022231.2740-2-pa...@netfilter.org>
TO: Pablo Neira Ayuso 
CC: netdev@vger.kernel.org

Hi Pablo,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]
[also build test WARNING on next-20181130]
[cannot apply to v4.20-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Pablo-Neira-Ayuso/add-flow_rule-infrastructure/20181130-204709
:: branch date: 14 hours ago
:: commit date: 14 hours ago

>> net/sched/cls_flower.c:1586:7-9: ERROR: reference preceded by free on line 
>> 1573

# 
https://github.com/0day-ci/linux/commit/012ba7afb48db31be34a84d5fe82c4ceb409fd2d
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 012ba7afb48db31be34a84d5fe82c4ceb409fd2d
vim +1586 net/sched/cls_flower.c

34738452 Jiri Pirko2018-07-23  1544
b95ec7eb Jiri Pirko2018-07-23  1545  static void 
*fl_tmplt_create(struct net *net, struct tcf_chain *chain,
b95ec7eb Jiri Pirko2018-07-23  1546  struct 
nlattr **tca,
b95ec7eb Jiri Pirko2018-07-23  1547  struct 
netlink_ext_ack *extack)
b95ec7eb Jiri Pirko2018-07-23  1548  {
b95ec7eb Jiri Pirko2018-07-23  1549 struct fl_flow_tmplt *tmplt;
b95ec7eb Jiri Pirko2018-07-23  1550 struct nlattr **tb;
b95ec7eb Jiri Pirko2018-07-23  1551 int err;
b95ec7eb Jiri Pirko2018-07-23  1552
b95ec7eb Jiri Pirko2018-07-23  1553 if (!tca[TCA_OPTIONS])
b95ec7eb Jiri Pirko2018-07-23  1554 return ERR_PTR(-EINVAL);
b95ec7eb Jiri Pirko2018-07-23  1555
b95ec7eb Jiri Pirko2018-07-23  1556 tb = kcalloc(TCA_FLOWER_MAX + 
1, sizeof(struct nlattr *), GFP_KERNEL);
b95ec7eb Jiri Pirko2018-07-23  1557 if (!tb)
b95ec7eb Jiri Pirko2018-07-23  1558 return 
ERR_PTR(-ENOBUFS);
b95ec7eb Jiri Pirko2018-07-23  1559 err = nla_parse_nested(tb, 
TCA_FLOWER_MAX, tca[TCA_OPTIONS],
b95ec7eb Jiri Pirko2018-07-23  1560
fl_policy, NULL);
b95ec7eb Jiri Pirko2018-07-23  1561 if (err)
b95ec7eb Jiri Pirko2018-07-23  1562 goto errout_tb;
b95ec7eb Jiri Pirko2018-07-23  1563
b95ec7eb Jiri Pirko2018-07-23  1564 tmplt = kzalloc(sizeof(*tmplt), 
GFP_KERNEL);
1cbc36a5 Dan Carpenter 2018-08-03  1565 if (!tmplt) {
1cbc36a5 Dan Carpenter 2018-08-03  1566 err = -ENOMEM;
b95ec7eb Jiri Pirko2018-07-23  1567 goto errout_tb;
1cbc36a5 Dan Carpenter 2018-08-03  1568 }
b95ec7eb Jiri Pirko2018-07-23  1569 tmplt->chain = chain;
b95ec7eb Jiri Pirko2018-07-23  1570 err = fl_set_key(net, tb, 
>dummy_key, >mask, extack);
b95ec7eb Jiri Pirko2018-07-23  1571 if (err)
b95ec7eb Jiri Pirko2018-07-23  1572 goto errout_tmplt;
b95ec7eb Jiri Pirko2018-07-23 @1573 kfree(tb);
b95ec7eb Jiri Pirko2018-07-23  1574
b95ec7eb Jiri Pirko2018-07-23  1575 
fl_init_dissector(>dissector, >mask);
b95ec7eb Jiri Pirko2018-07-23  1576
012ba7af Pablo Neira Ayuso 2018-11-29  1577 err = fl_hw_create_tmplt(chain, 
tmplt);
012ba7af Pablo Neira Ayuso 2018-11-29  1578 if (err)
012ba7af Pablo Neira Ayuso 2018-11-29  1579 goto errout_tmplt;
34738452 Jiri Pirko2018-07-23  1580
b95ec7eb Jiri Pirko2018-07-23  1581 return tmplt;
b95ec7eb Jiri Pirko2018-07-23  1582
b95ec7eb Jiri Pirko2018-07-23  1583  errout_tmplt:
b95ec7eb Jiri Pirko2018-07-23  1584 kfree(tmplt);
b95ec7eb Jiri Pirko2018-07-23  1585  errout_tb:
b95ec7eb Jiri Pirko2018-07-23 @1586 kfree(tb);
b95ec7eb Jiri Pirko2018-07-23  1587 return ERR_PTR(err);
b95ec7eb Jiri Pirko2018-07-23  1588  }
b95ec7eb Jiri Pirko2018-07-23  1589

:: The code at line 1586 was first introduced by commit
:: b95ec7eb3b4d2f158dd15c912cf670b546f09571 net: sched: cls_flower: 
implement chain templates

:: TO: Jiri Pirko 
:: CC: David S. Miller 

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

[Julia Lawall]

On Mon, 27 Aug 2018, Al Viro wrote:

> On Sun, Aug 26, 2018 at 11:35:17PM -0400, Julia Lawall wrote:
>
> > * x = \(kmalloc\|kzalloc\|devm_kmalloc\|devm_kzalloc\)(...)
>
> I can name several you've missed right off the top of my head -
> vmalloc, kvmalloc, kmem_cache_alloc, kmem_cache_zalloc, variants
> with _trace slapped on, and that is not to mention the things like
> get_free_page or

OK, maybe for a given type the set of functions would be smaller.

>
> void *my_k3wl_alloc(u64 n) // 'cause all artificial limits suck, that's why
> {
>   lots and lots of home-grown stats collection
>   some tracepoints thrown in just for fun
>   return kmalloc(n);
> }
>
> (and no, I'm not implying that net/sched folks had done anything of that
> sort; I have seen that and worse in drivers, though)
>
> > The * at the beginning of the line means to highlight what you are looking
> > for, which is done by making a diff in which the highlighted line
> > appears to be removed.
>
> Umm...  Does that cover return, BTW?  Or something like
>   T *barf;
>   extern void foo(T *p);
>   foo(kmalloc(sizeof(*barf)));

It only covers the pattern that is shown, ie an assignment.  For this,
another pattern would be needed.  It would be necessary to match first the
call that one is concerned with and then go find the function definition
or prototype to find the type of the associated parameter.  It is possible
to count the offset of the kmalloc call in the argument list and then get
the type at the corresponding offset in the parameter list of the function
declaration or prototype.

>
>
> > The limitation is the ability to figure out the type of x.  If it is a
> > local variable, Coccinelle should have no problem.  If it is a structure
> > field, it may be necessary to provide command line arguments like
> >
> > --all-includes --include-headers-for-types
> >
> > --all-includes means to try to find all include files that are mentioned
> > in the .c file.  The next stronger option is --recursive includes, which
> > means include what all of the mentioned files include as well,
> > recursively.  This tends to cause a major performance hit, because a lot
> > of code is being parsed.  --include-headers-for-types heals a bit with
> > that, as it only considers the header files when computing type
> > information, and now when applying the rules.
> >
> > With respect to ifdefs around variable declarations and structure field
> > declaration, in these cases Coccinelle considers that it cannot make the
> > ifdef have an if-like control flow, and so if considers the #ifdef, #else
> > and #endif to be comments.  Thus it takes into account only the last type
> > provided for a given variable.
>
> [snip]
>
> What about several variants of structure definition?  Because ifdefs around
> includes do occur in the wild...

Such ifdefs would be ignored completely.  I suspect that only the last
definition of the structure would be taken into account.

julia

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

[Julia Lawall]

On Mon, 27 Aug 2018, Al Viro wrote:

> On Sun, Aug 26, 2018 at 10:00:46PM -0400, Julia Lawall wrote:
> >
> >
> > On Sun, 26 Aug 2018, Al Viro wrote:
> >
> > > On Sun, Aug 26, 2018 at 03:26:54PM -0700, Joe Perches wrote:
> > > > On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote:
> > > > > On Sun, Aug 26, 2018 at 11:57:57AM -0700, Joe Perches wrote:
> > > > >
> > > > > > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc.
> > > > > > > arguments.  typeof is even worse in that respect.
> > > > > >
> > > > > > True.  Semantic searches via tools like coccinelle could help here
> > > > > > but those searches are quite a bit slower than straightforward 
> > > > > > greps.
> > > > >
> > > > > Those searches are .config-sensitive as well, which can be much more
> > > > > unpleasant than being slow...
> > > >
> > > > Are they?  Julia?
> > >
> > > They work pretty much on preprocessor output level; if something it 
> > > ifdef'ed
> > > out on given config, it won't be seen...
> >
> > Coccinelle doesn't care what is ifdef'd out.  It only misses the things it
> > can't parse.  Very strange ifdefs could indeed cause that, but it should
> > be a minor problem.
>
> OK, but... what does it do when it sees two definitions of a structure
> in different branches of #if/#else/#endif?  I think I'm confused about
> what it can and cannot do; to restate the original problem:
>   * we need to find all places where instances of given type
> are created.  Assume it never is a member of struct/union/array and
> no static or auto duration instances exist - everything is dynamically
> allocated somewhere.
>
> Can coccinelle do that and if it can, what are the limitations?

Looking at the thread, it seems that what you are asking for is something
like:

@@
struct tcf_proto *x;
@@

* x = \(kmalloc\|kzalloc\|devm_kmalloc\|devm_kzalloc\)(...)

The * at the beginning of the line means to highlight what you are looking
for, which is done by making a diff in which the highlighted line
appears to be removed.

The limitation is the ability to figure out the type of x.  If it is a
local variable, Coccinelle should have no problem.  If it is a structure
field, it may be necessary to provide command line arguments like

--all-includes --include-headers-for-types

--all-includes means to try to find all include files that are mentioned
in the .c file.  The next stronger option is --recursive includes, which
means include what all of the mentioned files include as well,
recursively.  This tends to cause a major performance hit, because a lot
of code is being parsed.  --include-headers-for-types heals a bit with
that, as it only considers the header files when computing type
information, and now when applying the rules.

With respect to ifdefs around variable declarations and structure field
declaration, in these cases Coccinelle considers that it cannot make the
ifdef have an if-like control flow, and so if considers the #ifdef, #else
and #endif to be comments.  Thus it takes into account only the last type
provided for a given variable.  For example, in the following:

int main() {
#ifdef X
  int x;
#else
  char xx;
#endif

  return x;
}

int main2() {
#ifdef X
  char x;
#else
  int x;
#endif

  return x;
}

x is considered to have type int in both returns.  But if xx is replaced
by x in the definition of main, then x at the point of the return will
have type char.

Around a statement, such as x = kmalloc(...), it should be able to
consider that both branches of an ifdef are possible.

But there are no absolute guarantees.  If there is any problem in parsing
a line of some function, the whole function will be ignores.

julia

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

[Julia Lawall]

On Sun, 26 Aug 2018, Al Viro wrote:

> On Sun, Aug 26, 2018 at 03:26:54PM -0700, Joe Perches wrote:
> > On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote:
> > > On Sun, Aug 26, 2018 at 11:57:57AM -0700, Joe Perches wrote:
> > >
> > > > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc.
> > > > > arguments.  typeof is even worse in that respect.
> > > >
> > > > True.  Semantic searches via tools like coccinelle could help here
> > > > but those searches are quite a bit slower than straightforward greps.
> > >
> > > Those searches are .config-sensitive as well, which can be much more
> > > unpleasant than being slow...
> >
> > Are they?  Julia?
>
> They work pretty much on preprocessor output level; if something it ifdef'ed
> out on given config, it won't be seen...

Coccinelle doesn't care what is ifdef'd out.  It only misses the things it
can't parse.  Very strange ifdefs could indeed cause that, but it should
be a minor problem.

julia

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

[Julia Lawall]

On Sun, 26 Aug 2018, Joe Perches wrote:

> On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote:
> > On Sun, Aug 26, 2018 at 11:57:57AM -0700, Joe Perches wrote:
> >
> > > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc.
> > > > arguments.  typeof is even worse in that respect.
> > >
> > > True.  Semantic searches via tools like coccinelle could help here
> > > but those searches are quite a bit slower than straightforward greps.
> >
> > Those searches are .config-sensitive as well, which can be much more
> > unpleasant than being slow...
>
> Are they?  Julia?

I don't completely understand the question.  Coccinelle doens't know
anything about the configuration.

julia

[PATCH] mwifiex: delete unneeded include

[Julia Lawall]

Nothing that is defined in 11ac.h is referenced in cmdevt.c.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c 
b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 7014f44..9cfcdf6 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -25,7 +25,6 @@
 #include "main.h"
 #include "wmm.h"
 #include "11n.h"
-#include "11ac.h"
 
 static void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
 

Re: [PATCH V2] net/netlink: optimize seq_puts and seq_printf in af_netlink.c

[Julia Lawall]

On Thu, 3 May 2018, YU Bo wrote:

> Before the patch, the command `cat /proc/net/netlink` will output like:
>
> https://clbin.com/BojZv
>
> After the patch:
>
> https://clbin.com/lnu4L
>
> The optimization will make convenience for using `cat /proc/net/netlink`
> But,The checkpatch will give a warning:
>
> WARNING: quoted string split across lines

The interest of the checkpatch warning is that someone may want to grep
for something that has actually been split over two lines.  If this is not
an issue in your case and if there are good reasons for splitting the
string, then you can ignore checkpatch.

julia

>
> Signed-off-by: Bo YU 
> ---
> Changes in v2:
> Do not break the indentation of the code line
> ---
> net/netlink/af_netlink.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
> index 55342c4d5cec..2e2dd88fc79f 100644
> --- a/net/netlink/af_netlink.c
> +++ b/net/netlink/af_netlink.c
> @@ -2606,13 +2606,13 @@ static int netlink_seq_show(struct seq_file *seq, void
> *v)
> {
>   if (v == SEQ_START_TOKEN) {
>   seq_puts(seq,
> -  "sk   Eth PidGroups   "
> -  "Rmem Wmem Dump Locks Drops
> Inode\n");
> +  "sk   Eth PidGroups   "
> +  "Rmem Wmem Dump  LocksDropsInode\n");
>   } else {
>   struct sock *s = v;
>   struct netlink_sock *nlk = nlk_sk(s);
>
> - seq_printf(seq, "%pK %-3d %-6u %08x %-8d %-8d %d %-8d %-8d
> %-8lu\n",
> + seq_printf(seq, "%pK %-3d %-10u %08x %-8d %-8d %-5d %-8d %-8d
> %-8lu\n",
>  s,
>  s->sk_protocol,
>  nlk->portid,
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit (fwd)

[Julia Lawall]

Line 1814 frees something that is dereferenced on the next line.

julia

-- Forwarded message --
Date: Tue, 17 Apr 2018 10:32:17 +0800
From: kbuild test robot <l...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit

CC: kbuild-...@01.org
In-Reply-To: <1523902550-10767-3-git-send-email-yihung@gmail.com>
References: <1523902550-10767-3-git-send-email-yihung@gmail.com>
TO: Yi-Hung Wei <yihung@gmail.com>
CC: netdev@vger.kernel.org
CC: Yi-Hung Wei <yihung@gmail.com>

Hi Yi-Hung,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Yi-Hung-Wei/openvswitch-Support-conntrack-zone-limit/20180417-085035
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> net/openvswitch/conntrack.c:1815:17-39: ERROR: reference preceded by free on 
>> line 1814

# 
https://github.com/0day-ci/linux/commit/01487f05f032565b952e344abace672a12045d9e
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 01487f05f032565b952e344abace672a12045d9e
vim +1815 net/openvswitch/conntrack.c

c2ac6673 Joe Stringer 2015-08-26  1786
01487f05 Yi-Hung Wei  2018-04-16  1787  #if 
IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
01487f05 Yi-Hung Wei  2018-04-16  1788  static int ovs_ct_limit_init(struct net 
*net, struct ovs_net *ovs_net)
01487f05 Yi-Hung Wei  2018-04-16  1789  {
01487f05 Yi-Hung Wei  2018-04-16  1790  int i;
01487f05 Yi-Hung Wei  2018-04-16  1791
01487f05 Yi-Hung Wei  2018-04-16  1792  ovs_net->ct_limit_info = 
kmalloc(sizeof *ovs_net->ct_limit_info,
01487f05 Yi-Hung Wei  2018-04-16  1793  
 GFP_KERNEL);
01487f05 Yi-Hung Wei  2018-04-16  1794  if (!ovs_net->ct_limit_info)
01487f05 Yi-Hung Wei  2018-04-16  1795  return -ENOMEM;
01487f05 Yi-Hung Wei  2018-04-16  1796
01487f05 Yi-Hung Wei  2018-04-16  1797  
ovs_net->ct_limit_info->default_limit = OVS_CT_LIMIT_DEFAULT;
01487f05 Yi-Hung Wei  2018-04-16  1798  ovs_net->ct_limit_info->limits =
01487f05 Yi-Hung Wei  2018-04-16  1799  
kmalloc_array(CT_LIMIT_HASH_BUCKETS, sizeof(struct hlist_head),
01487f05 Yi-Hung Wei  2018-04-16  1800
GFP_KERNEL);
01487f05 Yi-Hung Wei  2018-04-16  1801  if 
(!ovs_net->ct_limit_info->limits) {
01487f05 Yi-Hung Wei  2018-04-16  1802  
kfree(ovs_net->ct_limit_info);
01487f05 Yi-Hung Wei  2018-04-16  1803  return -ENOMEM;
01487f05 Yi-Hung Wei  2018-04-16  1804  }
01487f05 Yi-Hung Wei  2018-04-16  1805
01487f05 Yi-Hung Wei  2018-04-16  1806  for (i = 0; i < 
CT_LIMIT_HASH_BUCKETS; i++)
01487f05 Yi-Hung Wei  2018-04-16  1807  
INIT_HLIST_HEAD(_net->ct_limit_info->limits[i]);
01487f05 Yi-Hung Wei  2018-04-16  1808
01487f05 Yi-Hung Wei  2018-04-16  1809  ovs_net->ct_limit_info->data =
01487f05 Yi-Hung Wei  2018-04-16  1810  nf_conncount_init(net, 
NFPROTO_INET, sizeof(u32));
01487f05 Yi-Hung Wei  2018-04-16  1811
01487f05 Yi-Hung Wei  2018-04-16  1812  if 
(IS_ERR(ovs_net->ct_limit_info->data)) {
01487f05 Yi-Hung Wei  2018-04-16  1813  
kfree(ovs_net->ct_limit_info->limits);
01487f05 Yi-Hung Wei  2018-04-16 @1814  
kfree(ovs_net->ct_limit_info);
01487f05 Yi-Hung Wei  2018-04-16 @1815  return 
PTR_ERR(ovs_net->ct_limit_info->data);
01487f05 Yi-Hung Wei  2018-04-16  1816  }
01487f05 Yi-Hung Wei  2018-04-16  1817  return 0;
01487f05 Yi-Hung Wei  2018-04-16  1818  }
01487f05 Yi-Hung Wei  2018-04-16  1819

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 08/12] mmc: reduce use of block bounce buffers (fwd)

[Julia Lawall]

There is a duplicated test on line 360.

julia

-- Forwarded message --
Date: Mon, 16 Apr 2018 23:04:18 +0800
From: kbuild test robot <l...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 08/12] mmc: reduce use of block bounce buffers

CC: kbuild-...@01.org
In-Reply-To: <20180416085032.7367-9-...@lst.de>
References: <20180416085032.7367-9-...@lst.de>
TO: Christoph Hellwig <h...@lst.de>
CC: io...@lists.linux-foundation.org, linux-a...@vger.kernel.org, 
linux-bl...@vger.kernel.org, linux-...@vger.kernel.org, 
linux-s...@vger.kernel.org, netdev@vger.kernel.org
CC: linux-ker...@vger.kernel.org

Hi Christoph,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17-rc1 next-20180416]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Christoph-Hellwig/iscsi_tcp-don-t-set-a-bounce-limit/20180416-172618
:: branch date: 6 hours ago
:: commit date: 6 hours ago

>> drivers/mmc/core/queue.c:360:5-29: duplicated argument to && or ||

# 
https://github.com/0day-ci/linux/commit/6620a69f0eea8e8b7586f08f721c95a336022497
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 6620a69f0eea8e8b7586f08f721c95a336022497
vim +360 drivers/mmc/core/queue.c

81196976 Adrian Hunter 2017-11-29  350
c8b5fd03 Adrian Hunter 2017-09-22  351  static void mmc_setup_queue(struct 
mmc_queue *mq, struct mmc_card *card)
c8b5fd03 Adrian Hunter 2017-09-22  352  {
c8b5fd03 Adrian Hunter 2017-09-22  353  struct mmc_host *host = 
card->host;
c8b5fd03 Adrian Hunter 2017-09-22  354
8b904b5b Bart Van Assche   2018-03-07  355  
blk_queue_flag_set(QUEUE_FLAG_NONROT, mq->queue);
8b904b5b Bart Van Assche   2018-03-07  356  
blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, mq->queue);
c8b5fd03 Adrian Hunter 2017-09-22  357  if (mmc_can_erase(card))
c8b5fd03 Adrian Hunter 2017-09-22  358  
mmc_queue_setup_discard(mq->queue, card);
c8b5fd03 Adrian Hunter 2017-09-22  359
6620a69f Christoph Hellwig 2018-04-16 @360  if (!mmc_dev(host)->dma_mask || 
!mmc_dev(host)->dma_mask)
6620a69f Christoph Hellwig 2018-04-16  361  
blk_queue_bounce_limit(mq->queue, BLK_BOUNCE_HIGH);
c8b5fd03 Adrian Hunter 2017-09-22  362  
blk_queue_max_hw_sectors(mq->queue,
c8b5fd03 Adrian Hunter 2017-09-22  363  
min(host->max_blk_count, host->max_req_size / 512));
c8b5fd03 Adrian Hunter 2017-09-22  364  
blk_queue_max_segments(mq->queue, host->max_segs);
c8b5fd03 Adrian Hunter 2017-09-22  365  
blk_queue_max_segment_size(mq->queue, host->max_seg_size);
c8b5fd03 Adrian Hunter 2017-09-22  366
1e8e55b6 Adrian Hunter 2017-11-29  367  INIT_WORK(>recovery_work, 
mmc_mq_recovery_handler);
81196976 Adrian Hunter 2017-11-29  368  INIT_WORK(>complete_work, 
mmc_blk_mq_complete_work);
81196976 Adrian Hunter 2017-11-29  369
81196976 Adrian Hunter 2017-11-29  370  mutex_init(>complete_lock);
81196976 Adrian Hunter 2017-11-29  371
81196976 Adrian Hunter 2017-11-29  372  init_waitqueue_head(>wait);
81196976 Adrian Hunter 2017-11-29  373  }
81196976 Adrian Hunter 2017-11-29  374

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH v11 crypto 01/12] tls: support for Inline tls record (fwd)

[Julia Lawall]

ctx is dereferenced on line 258 but has been freed on line 229.

julia

-- Forwarded message --
Date: Sun, 18 Mar 2018 18:05:25 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v11 crypto 01/12] tls: support for Inline tls record

CC: kbuild-...@01.org
In-Reply-To: <1521214661-28928-1-git-send-email-atul.gu...@chelsio.com>
References: <1521214661-28928-1-git-send-email-atul.gu...@chelsio.com>
TO: Atul Gupta <atul.gu...@chelsio.com>
CC: davejwat...@fb.com, da...@davemloft.net, herb...@gondor.apana.org.au, 
s...@queasysnail.net, sbri...@redhat.com, linux-cry...@vger.kernel.org, 
netdev@vger.kernel.org, ganes...@chelsio.com
CC: s...@queasysnail.net, sbri...@redhat.com, linux-cry...@vger.kernel.org, 
netdev@vger.kernel.org, ganes...@chelsio.com

Hi Atul,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v4.16-rc4]
[cannot apply to next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Atul-Gupta/tls-support-for-Inline-tls-record/20180318-162840
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> net/tls/tls_main.c:258:5-8: ERROR: reference preceded by free on line 229

# 
https://github.com/0day-ci/linux/commit/be47378786b9d9874dfc3ab57504565275c7b3ff
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout be47378786b9d9874dfc3ab57504565275c7b3ff
vim +258 net/tls/tls_main.c

3c4d75591 Dave Watson   2017-06-14  218
3c4d75591 Dave Watson   2017-06-14  219  static void tls_sk_proto_close(struct 
sock *sk, long timeout)
3c4d75591 Dave Watson   2017-06-14  220  {
3c4d75591 Dave Watson   2017-06-14  221 struct tls_context *ctx = 
tls_get_ctx(sk);
3c4d75591 Dave Watson   2017-06-14  222 long timeo = sock_sndtimeo(sk, 
0);
3c4d75591 Dave Watson   2017-06-14  223 void (*sk_proto_close)(struct 
sock *sk, long timeout);
3c4d75591 Dave Watson   2017-06-14  224
3c4d75591 Dave Watson   2017-06-14  225 lock_sock(sk);
ff45d820a Ilya Lesokhin 2017-11-13  226 sk_proto_close = 
ctx->sk_proto_close;
ff45d820a Ilya Lesokhin 2017-11-13  227
ff45d820a Ilya Lesokhin 2017-11-13  228 if (ctx->tx_conf == 
TLS_BASE_TX) {
ff45d820a Ilya Lesokhin 2017-11-13 @229 kfree(ctx);
ff45d820a Ilya Lesokhin 2017-11-13  230 goto skip_tx_cleanup;
ff45d820a Ilya Lesokhin 2017-11-13  231 }
3c4d75591 Dave Watson   2017-06-14  232
3c4d75591 Dave Watson   2017-06-14  233 if 
(!tls_complete_pending_work(sk, ctx, 0, ))
3c4d75591 Dave Watson   2017-06-14  234 
tls_handle_open_record(sk, 0);
3c4d75591 Dave Watson   2017-06-14  235
3c4d75591 Dave Watson   2017-06-14  236 if (ctx->partially_sent_record) 
{
3c4d75591 Dave Watson   2017-06-14  237 struct scatterlist *sg 
= ctx->partially_sent_record;
3c4d75591 Dave Watson   2017-06-14  238
3c4d75591 Dave Watson   2017-06-14  239 while (1) {
3c4d75591 Dave Watson   2017-06-14  240 
put_page(sg_page(sg));
3c4d75591 Dave Watson   2017-06-14  241 
sk_mem_uncharge(sk, sg->length);
3c4d75591 Dave Watson   2017-06-14  242
3c4d75591 Dave Watson   2017-06-14  243 if 
(sg_is_last(sg))
3c4d75591 Dave Watson   2017-06-14  244 break;
3c4d75591 Dave Watson   2017-06-14  245 sg++;
3c4d75591 Dave Watson   2017-06-14  246 }
3c4d75591 Dave Watson   2017-06-14  247 }
ff45d820a Ilya Lesokhin 2017-11-13  248
3c4d75591 Dave Watson   2017-06-14  249 kfree(ctx->rec_seq);
3c4d75591 Dave Watson   2017-06-14  250 kfree(ctx->iv);
3c4d75591 Dave Watson   2017-06-14  251
ff45d820a Ilya Lesokhin 2017-11-13  252 if (ctx->tx_conf == TLS_SW_TX)
ff45d820a Ilya Lesokhin 2017-11-13  253 
tls_sw_free_tx_resources(sk);
3c4d75591 Dave Watson   2017-06-14  254
ff45d820a Ilya Lesokhin 2017-11-13  255  skip_tx_cleanup:
3c4d75591 Dave Watson   2017-06-14  256 release_sock(sk);
3c4d75591 Dave Watson   2017-06-14  257 sk_proto_close(sk, timeout);
be4737878 Atul Gupta2018-03-16 @258 if (ctx->tx_conf == 
TLS_HW_RECORD)
be4737878 Atul Gupta2018-03-16  259 kfree(ctx);
3c4d75591 Dave Watson   2017-06-14  260  }
3c4d75591 Dave Watson   2017-06-14  261

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH v11 crypto 12/12] crypto: chtls - Makefile Kconfig (fwd)

[Julia Lawall]

Please check the indentation on line 1655.

thanks,
julia

-- Forwarded message --
Date: Sun, 18 Mar 2018 18:15:36 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v11 crypto 12/12] crypto: chtls - Makefile Kconfig

CC: kbuild-...@01.org
In-Reply-To: <1521214661-28928-12-git-send-email-atul.gu...@chelsio.com>
References: <1521214661-28928-12-git-send-email-atul.gu...@chelsio.com>
TO: Atul Gupta <atul.gu...@chelsio.com>
CC: davejwat...@fb.com, da...@davemloft.net, herb...@gondor.apana.org.au
CC: s...@queasysnail.net, sbri...@redhat.com, linux-cry...@vger.kernel.org, 
netdev@vger.kernel.org, ganes...@chelsio.com

Hi Atul,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[cannot apply to next-20180316]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Atul-Gupta/tls-support-for-Inline-tls-record/20180318-162840
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
:: branch date: 2 hours ago
:: commit date: 2 hours ago

All error/warnings (new ones prefixed by >>):

   drivers/crypto/chelsio/chtls/chtls_io.c: In function 'chtls_expansion_size':
>> drivers/crypto/chelsio/chtls/chtls_io.c:457:2: error: expected ',' or ';' 
>> before 'int'
 int expnsize, frcn, fraglast, fragsize;
 ^~~
>> drivers/crypto/chelsio/chtls/chtls_io.c:461:3: error: 'fragsize' undeclared 
>> (first use in this function); did you mean 'ivs_size'?
  fragsize = hws->mfs;
  ^~~~
  ivs_size
   drivers/crypto/chelsio/chtls/chtls_io.c:461:3: note: each undeclared 
identifier is reported only once for each function it appears in
>> drivers/crypto/chelsio/chtls/chtls_io.c:465:4: error: 'frcnt' undeclared 
>> (first use in this function); did you mean 'pducnt'?
   frcnt = (data_len / fragsize);
   ^
   pducnt
>> drivers/crypto/chelsio/chtls/chtls_io.c:468:4: error: 'expnsize' undeclared 
>> (first use in this function); did you mean 'fragsize'?
   expnsize =  frcnt * expppdu;
   ^~~~
   fragsize
>> drivers/crypto/chelsio/chtls/chtls_io.c:480:4: error: 'fraglast' undeclared 
>> (first use in this function); did you mean 'rb_last'?
   fraglast = data_len % fragsize;
   ^~~~
   rb_last
   drivers/crypto/chelsio/chtls/chtls_io.c: In function 'peekmsg':
>> drivers/crypto/chelsio/chtls/chtls_io.c:1653:5: warning: this 'if' clause 
>> does not guard... [-Wmisleading-indentation]
if (!copied)
^~
   drivers/crypto/chelsio/chtls/chtls_io.c:1655:6: note: ...this statement, but 
the latter is misleadingly indented as if it were guarded by the 'if'
 break;
 ^
   drivers/crypto/chelsio/chtls/chtls_io.c: In function 'chtls_expansion_size':
>> drivers/crypto/chelsio/chtls/chtls_io.c:492:1: warning: control reaches end 
>> of non-void function [-Wreturn-type]
}
^

coccinelle warnings: (new ones prefixed by >>)

>> drivers/crypto/chelsio/chtls/chtls_io.c:1654:5-22: code aligned with 
>> following code on line 1655

# 
https://github.com/0day-ci/linux/commit/635907fe348f84b525d7ce16ae8f2a9b82c631e3
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 635907fe348f84b525d7ce16ae8f2a9b82c631e3
vim +1654 drivers/crypto/chelsio/chtls/chtls_io.c

8ae18d74 Atul Gupta 2018-03-16  1542
8ae18d74 Atul Gupta 2018-03-16  1543  /*
8ae18d74 Atul Gupta 2018-03-16  1544   * Peek at data in a socket's receive 
buffer.
8ae18d74 Atul Gupta 2018-03-16  1545   */
8ae18d74 Atul Gupta 2018-03-16  1546  static int peekmsg(struct sock *sk, 
struct msghdr *msg,
8ae18d74 Atul Gupta 2018-03-16  1547   size_t len, int nonblock, 
int flags)
8ae18d74 Atul Gupta 2018-03-16  1548  {
8ae18d74 Atul Gupta 2018-03-16  1549struct tcp_sock *tp = tcp_sk(sk);
8ae18d74 Atul Gupta 2018-03-16  1550struct sk_buff *skb;
8ae18d74 Atul Gupta 2018-03-16  1551u32 peek_seq, offset;
8ae18d74 Atul Gupta 2018-03-16  1552int copied = 0;
8ae18d74 Atul Gupta 2018-03-16  1553size_t avail;  /* amount of 
available data in current skb */
8ae18d74 Atul Gupta 2018-03-16  1554long timeo;
8ae18d74 Atul Gupta 2018-03-16  1555
8ae18d74 Atul Gupta 2018-03-16  1556lock_sock(sk);
8ae18d74 Atul Gupta 2018-03-16  1557timeo = sock_rcvtimeo(sk, nonblock);
8ae18d74 Atul Gupta 2018-03-16  1558peek_seq = tp->copied_seq;
8ae18d74 Atul Gupta 2018-03-16  1559
8ae18d74 Atul Gupta 2018-03-16  1560do {
8ae18d74 Atul Gupta 2018-03-16  1561if (unlikely(tp->urg_data && 
tp->urg_seq == peek_se

Re: [Outreachy kernel] Re: [PATCH v2] net: ethernet: Drop unnecessary continue

[Julia Lawall]

On Wed, 7 Mar 2018, Arushi Singhal wrote:

>
>
> On Wed, Mar 7, 2018 at 9:09 PM, David Miller  wrote:
>   From: Arushi Singhal 
>   Date: Sat, 3 Mar 2018 21:44:56 +0530
>
>   > Continue at the bottom of a loop are removed.
>   > Issue found using drop_continue.cocci Coccinelle script.
>   >
>   > Signed-off-by: Arushi Singhal
>   
>   > ---
>   > Changes in v2:
>   > - Braces is dropped from if with single statement.
>
>   Sorry there is no way I am applying this.
>
>   Just blindly removing continue statements that some static
>   checker
>   or compiler warning mentions is completely unacceptable.
>
>   Actually _LOOK_ at this code:
>
>   >               if(cards[i].vendor_id) {
>   >                       for(j=0;j<3;j++)
>   > -                           
>    if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
>   {
>   > +                           
>    if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
>   >                                       release_region(ioaddr,
>   cards[i].total_size);
>   > -                                     continue;
>   > -                       }
>   >               }
>
>   The extraneous continue statement is the _least_ of the problems
>   here.
>
>
> Hello David
>
> Yes you are correct.
>  
>
>   This code, if the if() statement triggers, will release the
>   ioaddr
>   region and then _CONTINUE_ the for(j) loop, and try to access
>   the
>   registers using the same 'ioaddr' again.
>
>   That's actually a _REAL_ bug, and you're making it seem like
>   the behavior is intentional by editing it like this.
>
>   And the bug probably exists because this entire sequence has
>   misaligned closing curly braces.  It makes it look like
>   the continue is for the outer loop, which would be fine.
>
>   But it's not.  It's for the inner loop, and this causes the "use
>   ioaddr after releasing it" bug.
>
>
> Yes I know it's for the inner loop.
> But I am not able to find, where I am wrong here

Arushi,

You are preserving the current behavior and David is telling you that the
current behavior is incorrect.  He doesn't want a patch that changes the
code but preesrves the current (wrong) behavior, because that somehow
contributes to the illusion that the incorrect code is correct.

julia

>
> For example
> BEFORE:-
> for(i=1;i<10;i++)
>    if (expr1)
>   {
>  statement;
>  continue;
>   }
>  
> After My Patch
> for(i=1;i<10;i++)
>     if (expr1)
>    statement;
>
> I am not understanding where I am getting wrong in understanding this.
> For me both are equivalent.
>
> I Agree with Jakub reply:-
> "This is an error handling path so the continue makes sense here to
> indicate the processing can't ever fall through if more statements are
> ever added to the loop.  But OK."
>
> Thanks
> Arushi
>
>
>   Please do not submit patches like this one, it makes for a lot
>   of
>   wasted auditing and review for people.  The onus is on you to
>   read
>   and understand the code you are editing before submitting your
>   changes.
>
>   Thank you.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web 
> visithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF8_axeTnahPmokxm
> Dnm4NmWo8QWSEmLejN4fO1nAiDaBA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] [PATCH v3] staging: ipx: Replace printk() with appropriate net_*macro_ratelimited()

[Julia Lawall]

On Mon, 5 Mar 2018, Arushi Singhal wrote:

> Replace printk having a log level with the appropriate
> net_*macro_ratelimited.
> It's better to use actual device name as a prefix in error messages.
>
> Signed-off-by: Arushi Singhal 
> ---
> changes in v2
> *In v1 printk was changed to pr_*macro(), which is used
> in kernel instead of calling printk() directly. And for drivers,
> dev_*macro() or net_*macro_ratelimited() should be used for calling
> printk() directly.
>
> changes in v3
> *Indentation is not changed, as line is exceeding 80 characters limit.

Put the v3 changes on top of the v2 changes, so that one can see
immediately what has changed most recently.

julia

>
>  drivers/staging/ipx/af_ipx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/ipx/af_ipx.c b/drivers/staging/ipx/af_ipx.c
> index d21a9d1..5ec6591 100644
> --- a/drivers/staging/ipx/af_ipx.c
> +++ b/drivers/staging/ipx/af_ipx.c
> @@ -744,7 +744,7 @@ static void ipxitf_discover_netnum(struct ipx_interface 
> *intrfc,
>   intrfc->if_netnum = cb->ipx_source_net;
>   ipxitf_add_local_route(intrfc);
>   } else {
> - printk(KERN_WARNING "IPX: Network number collision "
> + net_warn_ratelimited("IPX: Network number collision "
>   "%lx\n%s %s and %s %s\n",
>   (unsigned long) ntohl(cb->ipx_source_net),
>   ipx_device_name(i),
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20180305041740.GA23378%40seema-Inspiron-15-3567.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v2] staging: Replace printk() with appropriate net_*macro_ratelimited()

[Julia Lawall]

On Mon, 5 Mar 2018, Arushi Singhal wrote:

> Replace printk having a log level with the appropriate
> net_*macro_ratelimited.

Why did you choose this function?

> It's better to use actual device name as a prefix in error messages.

What does this message relate to.

> Indentation is also changed, to fix the  checkpatch issue.

It would be better to no exceed 80 characters than to follow the
suggestion abotu the argument being to the right of the (.

julia


> Signed-off-by: Arushi Singhal 
> ---
> changes in v2
> *In previous version printk was changed to pr_*macro(), which is used
> in kernel instead of calling printk() directly. And for drivers,
> dev_*macro() or net_*macro_ratelimited() should be used for calling
> printk() directly.
>
>  drivers/staging/ipx/af_ipx.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/ipx/af_ipx.c b/drivers/staging/ipx/af_ipx.c
> index d21a9d1..9a96962 100644
> --- a/drivers/staging/ipx/af_ipx.c
> +++ b/drivers/staging/ipx/af_ipx.c
> @@ -744,13 +744,13 @@ static void ipxitf_discover_netnum(struct ipx_interface 
> *intrfc,
>   intrfc->if_netnum = cb->ipx_source_net;
>   ipxitf_add_local_route(intrfc);
>   } else {
> - printk(KERN_WARNING "IPX: Network number collision "
> - "%lx\n%s %s and %s %s\n",
> - (unsigned long) ntohl(cb->ipx_source_net),
> - ipx_device_name(i),
> - ipx_frame_name(i->if_dlink_type),
> - ipx_device_name(intrfc),
> - ipx_frame_name(intrfc->if_dlink_type));
> + net_warn_ratelimited("IPX: Network number collision "
> +  "%lx\n%s %s and %s %s\n",
> +  (unsigned long) 
> ntohl(cb->ipx_source_net),
> +  ipx_device_name(i),
> +  ipx_frame_name(i->if_dlink_type),
> +  ipx_device_name(intrfc),
> +  
> ipx_frame_name(intrfc->if_dlink_type));
>   ipxitf_put(i);
>   }
>   }
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20180304204910.GA4840%40seema-Inspiron-15-3567.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] net: ethernet: Drop unnecessary continue

[Julia Lawall]

On Sat, 3 Mar 2018, Arushi Singhal wrote:

> Continue at the bottom of a loop are removed.
> Issue found using drop_continue.cocci Coccinelle script.

In each case you leave an if with a single statement in the branch.  In
that case the { } should be dropped too.

julia

>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/net/ethernet/amd/ni65.c   | 1 -
>  drivers/net/ethernet/neterion/s2io.c  | 1 -
>  drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 1 -
>  3 files changed, 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/ni65.c b/drivers/net/ethernet/amd/ni65.c
> index e248d1a..5975f29 100644
> --- a/drivers/net/ethernet/amd/ni65.c
> +++ b/drivers/net/ethernet/amd/ni65.c
> @@ -437,7 +437,6 @@ static int __init ni65_probe1(struct net_device *dev,int 
> ioaddr)
>   for(j=0;j<3;j++)
>   if(inb(ioaddr+cards[i].addr_offset+j) != 
> cards[i].vendor_id[j]) {
>   release_region(ioaddr, 
> cards[i].total_size);
> - continue;
> }
>   }
>   break;
> diff --git a/drivers/net/ethernet/neterion/s2io.c 
> b/drivers/net/ethernet/neterion/s2io.c
> index b8983e7..5123abd 100644
> --- a/drivers/net/ethernet/neterion/s2io.c
> +++ b/drivers/net/ethernet/neterion/s2io.c
> @@ -3682,7 +3682,6 @@ static void restore_xmsi_data(struct s2io_nic *nic)
>   if (wait_for_msix_trans(nic, msix_index)) {
>   DBG_PRINT(ERR_DBG, "%s: index: %d failed\n",
> __func__, msix_index);
> - continue;
>   }
>   }
>  }
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c 
> b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> index 15fa47f..77916ed 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
> @@ -260,7 +260,6 @@ nfp_net_pf_alloc_vnics(struct nfp_pf *pf, void __iomem 
> *ctrl_bar,
>   /* Kill the vNIC if app init marked it as invalid */
>   if (nn->port && nn->port->type == NFP_PORT_INVALID) {
>   nfp_net_pf_free_vnic(pf, nn);
> - continue;
>   }
>   }
>
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20180303130933.GA8337%40seema-Inspiron-15-3567.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

[Julia Lawall]

On Tue, 6 Feb 2018, Dan Carpenter wrote:

> That found 4 that I think Wolfram's grep missed.
>
>  arch/um/drivers/vector_user.h |2 --
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h|2 --
>  drivers/video/fbdev/mxsfb.c   |2 --
>  include/drm/drm_scdc_helper.h |3 ---
>
> But it didn't find the two bugs that Geert found where the right side
> wasn't a number literal.
>
> drivers/net/can/m_can/m_can.c:#define RXFC_FWM_MASK (0x7f < 
> RXFC_FWM_SHIFT)

OK, I can easily add this in - I've got rules to protect against reporting
it at the moment.  It may end up with false positives.

> drivers/usb/gadget/udc/goku_udc.h:#define INT_EPnNAK(n) (0x00100 < (n))   
>   /* 0 < n < 4 */

This is indeed harder, because one has to look at the usage site.

julia

Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

[Julia Lawall]

On Tue, 6 Feb 2018, Wolfram Sang wrote:

> Hi Julia,
>
> > and got the results below.  I can make a version for the kernel shortly.
>
> It should probably take care of right-shifting, too?

I did that too but got no results.  Perhaps right shifting constants is
pretty uncommon.  I can put that in the complete rule though.

julia

Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

[Julia Lawall]

On Tue, 6 Feb 2018, Dan Carpenter wrote:

> On Tue, Feb 06, 2018 at 02:15:51PM +0100, Julia Lawall wrote:
> >
> >
> > On Tue, 6 Feb 2018, Dan Carpenter wrote:
> >
> > > On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote:
> > > > In one Renesas driver, I found a typo which turned an intended bit 
> > > > shift ('<<')
> > > > into a comparison ('<'). Because this is a subtle issue, I looked tree 
> > > > wide for
> > > > similar patterns. This small patch series is the outcome.
> > > >
> > > > Buildbot and checkpatch are happy. Only compile-tested. To be applied
> > > > individually per sub-system, I think. I'd think only the net: amd: 
> > > > patch needs
> > > > to be conisdered for stable, but I leave this to people who actually 
> > > > know this
> > > > driver.
> > > >
> > > > CCing Dan. Maybe he has an idea how to add a test to smatch? In my 
> > > > setup, only
> > > > cppcheck reported a 'coding style' issue with a low prio.
> > > >
> > >
> > > Most of these are inside macros so it makes it complicated for Smatch
> > > to warn about them.  It might be easier in Coccinelle.  Julia the bugs
> > > look like this:
> > >
> > > - reissue_mask |= 0x < 4;
> > > + reissue_mask |= 0x << 4;
> >
> > Thanks.  I'll take a look.  Do you have an example of the macro issue
> > handy?
> >
>
> It's the same:
>
> #define EXYNOS_CIIMGEFF_PAT_CBCR_MASK  ((0xff < 13) | (0xff < 0))
>
> Smatch only sees the outside of the macro (where it is used in the code)
> and the pre-processed code.

I wrote the following rule:

@@
constant int x,y;
identifier i;
type T;
@@

(
i < x
|
x < i
|
 (T)i < x
|
x < (T)i
|
* x < y
)

and got the results below.  I can make a version for the kernel shortly.

julia

diff -u -p /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h 
/tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h
--- /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h
+++ /tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h
@@ -569,7 +569,6 @@
 #define EXYNOS_CIIMGEFF_FIN_EMBOSSING  (4 << 26)
 #define EXYNOS_CIIMGEFF_FIN_SILHOUETTE (5 << 26)
 #define EXYNOS_CIIMGEFF_FIN_MASK   (7 << 26)
-#define EXYNOS_CIIMGEFF_PAT_CBCR_MASK  ((0xff < 13) | (0xff < 0))

 /* Real input DMA size register */
 #define EXYNOS_CIREAL_ISIZE_AUTOLOAD_ENABLE(1 << 31)
diff -u -p /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c 
/tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
--- /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ /tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -595,7 +595,6 @@ isr_done:

reissue_mask = 1 << 0;
if (!pdata->per_channel_irq)
-   reissue_mask |= 0x < 4;

XP_IOWRITE(pdata, XP_INT_REISSUE_EN, reissue_mask);
}
diff -u -p /run/shm/linux-next/drivers/video/fbdev/mxsfb.c 
/tmp/nothing/drivers/video/fbdev/mxsfb.c
--- /run/shm/linux-next/drivers/video/fbdev/mxsfb.c
+++ /tmp/nothing/drivers/video/fbdev/mxsfb.c
@@ -133,8 +133,6 @@
 #define VDCTRL4_SYNC_SIGNALS_ON(1 << 18)
 #define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3)

-#define DEBUG0_HSYNC   (1 < 26)
-#define DEBUG0_VSYNC   (1 < 25)

 #define MIN_XRES   120
 #define MIN_YRES   120
diff -u -p /run/shm/linux-next/include/drm/drm_scdc_helper.h 
/tmp/nothing/include/drm/drm_scdc_helper.h
--- /run/shm/linux-next/include/drm/drm_scdc_helper.h
+++ /tmp/nothing/include/drm/drm_scdc_helper.h
@@ -50,9 +50,6 @@
 #define  SCDC_READ_REQUEST_ENABLE (1 << 0)

 #define SCDC_STATUS_FLAGS_0 0x40
-#define  SCDC_CH2_LOCK (1 < 3)
-#define  SCDC_CH1_LOCK (1 < 2)
-#define  SCDC_CH0_LOCK (1 < 1)
 #define  SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK)
 #define  SCDC_CLOCK_DETECT (1 << 0)

diff -u -p /run/shm/linux-next/arch/um/drivers/vector_user.h 
/tmp/nothing/arch/um/drivers/vector_user.h
--- /run/shm/linux-next/arch/um/drivers/vector_user.h
+++ /tmp/nothing/arch/um/drivers/vector_user.h
@@ -61,8 +61,6 @@ struct vector_fds {
 };

 #define VECTOR_READ1
-#define VECTOR_WRITE   (1 < 1)
-#define VECTOR_HEADERS (1 < 2)

 extern struct arglist *uml_parse_vector_ifspec(char *arg);

diff -u -p /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h 
/tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h
--- /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ /tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -

Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

[Julia Lawall]

On Tue, 6 Feb 2018, Dan Carpenter wrote:

> On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote:
> > In one Renesas driver, I found a typo which turned an intended bit shift 
> > ('<<')
> > into a comparison ('<'). Because this is a subtle issue, I looked tree wide 
> > for
> > similar patterns. This small patch series is the outcome.
> >
> > Buildbot and checkpatch are happy. Only compile-tested. To be applied
> > individually per sub-system, I think. I'd think only the net: amd: patch 
> > needs
> > to be conisdered for stable, but I leave this to people who actually know 
> > this
> > driver.
> >
> > CCing Dan. Maybe he has an idea how to add a test to smatch? In my setup, 
> > only
> > cppcheck reported a 'coding style' issue with a low prio.
> >
>
> Most of these are inside macros so it makes it complicated for Smatch
> to warn about them.  It might be easier in Coccinelle.  Julia the bugs
> look like this:
>
> - reissue_mask |= 0x < 4;
> + reissue_mask |= 0x << 4;

Thanks.  I'll take a look.  Do you have an example of the macro issue
handy?

julia

>
> regards,
> dan carpenter
>
> > Wolfram Sang (4):
> >   v4l: vsp1: fix mask creation for MULT_ALPHA_RATIO
> >   drm/exynos: fix comparison to bitshift when dealing with a mask
> >   v4l: dvb-frontends: stb0899: fix comparison to bitshift when dealing
> > with a mask
> >   net: amd-xgbe: fix comparison to bitshift when dealing with a mask
> >
> >  drivers/gpu/drm/exynos/regs-fimc.h| 2 +-
> >  drivers/media/dvb-frontends/stb0899_reg.h | 8 
> >  drivers/media/platform/vsp1/vsp1_regs.h   | 2 +-
> >  drivers/net/ethernet/amd/xgbe/xgbe-drv.c  | 2 +-
> >  4 files changed, 7 insertions(+), 7 deletions(-)
> >
> > --
> > 2.11.0
>

[PATCH] netfilter: nf_tables: fix odd_ptr_err.cocci warnings

[Julia Lawall]

tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   b4464bcab38d3f7fe995a7cb960eeac6889bec08
commit: 3b49e2e94e6ebb8b23d0955d9e898254455734f8 [8286/9035] netfilter:
nf_tables: add flow table netlink frontend

The following is a 0-day report generated by Coccinelle.  But from the
line before, it looks like the fix is backwards, and the test shoud be on
flowtable.

julia


 nf_tables_api.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -5419,7 +5419,7 @@ static int nf_tables_getflowtable(struct
flowtable = nf_tables_flowtable_lookup(table, nla[NFTA_FLOWTABLE_NAME],
   genmask);
if (IS_ERR(table))
-   return PTR_ERR(flowtable);
+   return PTR_ERR(table);

skb2 = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (!skb2)

Re: [PATCH] ethernet: mlx4: Delete an error message for a failed memory allocation in five functions

[Julia Lawall]

On Wed, 3 Jan 2018, Tariq Toukan wrote:

>
>
> On 01/01/2018 10:46 PM, SF Markus Elfring wrote:
> > From: Markus Elfring 
> > Date: Mon, 1 Jan 2018 21:42:27 +0100
> >
> > Omit an extra message for a memory allocation failure in these functions.
> >
> > This issue was detected by using the Coccinelle software.
> >
> > Signed-off-by: Markus Elfring 
> > ---
>
> Is this an issue? Why? What is your motivation?
> These are error messages, very informative, appear only upon errors, and in
> control flow.

Strings take up space.  Since there is a backtrace on an out of memory
problem, if the string does not provide any more information than the
position of the call, then there is not much added value.  I don't know
what was the string in this case.  If it provides some additional
information, then it would be reasonable to keep it.

julia

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

Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

[Julia Lawall]

On Tue, 2 Jan 2018, Bart Van Assche wrote:

> On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote:
> > On Tue, 2 Jan 2018, Bob Peterson wrote:
> > > - Original Message -
> > > > - Original Message -
> > > >
> > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages,
> > > and I don't like the thought of re-combining them all.
> >
> > Actually, the point of the patch was to remove the unnecessary \n at the
> > end of the string, because log_print will add another one.  If you prefer
> > to keep the string broken up, I can resend the patch in that form, but
> > without the unnecessary \n.
>
> Please combine any user-visible strings into a single line for which the
> unneeded newline is dropped since these strings are modified anyway by
> your patch.

That is what the submitted patch (2/12 specifically) did.

julia

Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

[Julia Lawall]

On Tue, 2 Jan 2018, Bob Peterson wrote:

> - Original Message -
> | - Original Message -
> | | Drop newline at the end of a message string when the printing function 
> adds
> | | a newline.
> |
> | Hi Julia,
> |
> | NACK.
> |
> | As much as it's a pain when searching the source code for output strings,
> | this patch set goes against the accepted Linux coding style document. See:
> |
> | 
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings
> |
> | Regards,
> |
> | Bob Peterson
> |
> |
> Hm. I guess I stand corrected. The document reads:
>
> "However, never break user-visible strings such as printk messages, because 
> that breaks the ability to grep for them."
>
> Still, the GFS2 and DLM code has a plethora of broken-up printk messages,
> and I don't like the thought of re-combining them all.

Actually, the point of the patch was to remove the unnecessary \n at the
end of the string, because log_print will add another one.  If you prefer
to keep the string broken up, I can resend the patch in that form, but
without the unnecessary \n.

julia

Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

[Julia Lawall]

On Tue, 2 Jan 2018, Bob Peterson wrote:

> - Original Message -
> | Drop newline at the end of a message string when the printing function adds
> | a newline.
>
> Hi Julia,
>
> NACK.
>
> As much as it's a pain when searching the source code for output strings,
> this patch set goes against the accepted Linux coding style document. See:
>
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings

I don't think that's the case:

"However, never break user-visible strings such as printk messages,
because that breaks the ability to grep for them."

julia

>
> Regards,
>
> Bob Peterson
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH 03/12] net: dccp: drop unneeded newline

[Julia Lawall]

DCCP_CRIT prints some other text and then a newline after the message
string, so the message string does not need to include a newline
explicitly.  Done using Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/dccp/ackvec.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index 3de0d03..2a24f7d 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -228,7 +228,7 @@ static void dccp_ackvec_add_new(struct dccp_ackvec *av, u32 
num_packets,
}
 
if (num_cells + dccp_ackvec_buflen(av) >= DCCPAV_MAX_ACKVEC_LEN) {
-   DCCP_CRIT("Ack Vector buffer overflow: dropping old entries\n");
+   DCCP_CRIT("Ack Vector buffer overflow: dropping old entries");
av->av_overflow = true;
}
 

[PATCH 05/12] openvswitch: drop unneeded newline

[Julia Lawall]

OVS_NLERR prints a newline at the end of the message string, so the
message string does not need to include a newline explicitly.  Done
using Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/openvswitch/conntrack.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index b27c5c6..62f36cc9 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1266,14 +1266,14 @@ static int parse_nat(const struct nlattr *attr,
/* Do not allow flags if no type is given. */
if (info->range.flags) {
OVS_NLERR(log,
- "NAT flags may be given only when NAT range 
(SRC or DST) is also specified.\n"
+ "NAT flags may be given only when NAT range 
(SRC or DST) is also specified."
  );
return -EINVAL;
}
info->nat = OVS_CT_NAT;   /* NAT existing connections. */
} else if (!info->commit) {
OVS_NLERR(log,
- "NAT attributes may be specified only when CT COMMIT 
flag is also specified.\n"
+ "NAT attributes may be specified only when CT COMMIT 
flag is also specified."
  );
return -EINVAL;
}

[PATCH 00/12] drop unneeded newline

[Julia Lawall]

Drop newline at the end of a message string when the printing function adds
a newline.

The complete semantic patch that detects this issue is as shown below
(http://coccinelle.lip6.fr/).  It works in two phases - the first phase
counts how many uses of a function involve a newline and how many don't,
and then the second phase removes newlines in the case of calls where a
newline is used one fourth of the times or less.

This approach is only moderately reliable, and all patches have been
checked to ensure that the newline is not needed.

This also converts some cases of string concatenation to single strings in
modified code, as this improves greppability.

// 
virtual after_start

@initialize:ocaml@
@@

let withnl = Hashtbl.create 101
let withoutnl = Hashtbl.create 101

let ignore =
  ["strcpy";"strlcpy";"strcat";"strlcat";"strcmp";"strncmp";"strcspn";
"strsep";"sprintf";"printf";"strncasecmp";"seq_printf";"strstr";"strspn";
"strlen";"strpbrk";"strtok_r";"memcmp";"memcpy"]

let dignore = ["tools";"samples"]

let inc tbl k =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  let cell = ref 0 in
  Hashtbl.add tbl k cell;
  cell in
  cell := 1 + !cell

let endnl c =
  let len = String.length c in
  try
String.get c (len-3) = '\\' && String.get c (len-2) = 'n' &&
String.get c (len-1) = '"'
  with _ -> false

let clean_string s extra =
  let pieces = Str.split (Str.regexp "\" \"") s in
  let nonempty s =
not (s = "") && String.get s 0 = '"' && not (String.get s 1 = '"') in
  let rec loop = function
  [] -> []
| [x] -> [x]
| x::y::rest ->
if nonempty x && nonempty y
then
  let xend = String.get x (String.length x - 2) = ' ' in
  let yend = String.get y 1 = ' ' in
  match (xend,yend) with
(true,false) | (false,true) -> x :: (loop (y::rest))
  | (true,true) ->
  x :: (loop (((String.sub y 0 (String.length y - 2))^"\"")::rest))
  | (false,false) ->
  ((String.sub x 0 (String.length x - 1)) ^ " \"") ::
  (loop (y::rest))
else x :: (loop (y::rest)) in
  (String.concat "" (loop pieces))^extra

@r depends on !after_start@
constant char[] c;
expression list[n] es;
identifier f;
position p;
@@

f@p(es,c,...)

@script:ocaml@
f << r.f;
n << r.n;
p << r.p;
c << r.c;
@@

let pieces = Str.split (Str.regexp "/") (List.hd p).file in
if not (List.mem f ignore) &&
  List.for_all (fun x -> not (List.mem x pieces)) dignore
then
  (if endnl c
  then inc withnl (f,n)
  else inc withoutnl (f,n))

@finalize:ocaml depends on !after_start@
w1 << merge.withnl;
w2 << merge.withoutnl;
@@

let names = ref [] in
let incn tbl k v =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  begin
let cell = ref 0 in
Hashtbl.add tbl k cell;
cell
  end in
  (if not (List.mem k !names) then names := k :: !names);
  cell := !v + !cell in
List.iter (function w -> Hashtbl.iter (incn withnl) w) w1;
List.iter (function w -> Hashtbl.iter (incn withoutnl) w) w2;

List.iter
  (function name ->
let wth = try !(Hashtbl.find withnl name) with _ -> 0 in
let wo = try !(Hashtbl.find withoutnl name) with _ -> 0 in
if wth > 0 && wth <= wo / 3 then Hashtbl.remove withnl name
else (Printf.eprintf "dropping %s %d %d\n" (fst name) wth wo; 
Hashtbl.remove withoutnl name; Hashtbl.remove withnl name))
  !names;

let it = new iteration() in
it#add_virtual_rule After_start;
it#register()

@s1 depends on after_start@
constant char[] c;
expression list[n] es;
identifier f;
position p;
@@

f(es,c@p,...)

@script:ocaml s2@
f << s1.f;
n << s1.n;
c << s1.c;
newc;
@@

try
  let _ = Hashtbl.find withnl (f,n) in
  if endnl c
  then Coccilib.include_match false
  else newc :=
make_expr(clean_string (String.sub c 0 (String.length c - 1)) "\\n\"")
with Not_found ->
try
  let _ = Hashtbl.find withoutnl (f,n) in
  if endnl c
  then newc :=
make_expr(clean_string (String.sub c 0 (String.length c - 3)) "\"")
  else Coccilib.include_match false
with Not_found -> Coccilib.include_match false

@@
constant char[] s1.c;
position s1.p;
expression s2.newc;
@@

- c@p
+ newc
// 

---

 arch/arm/mach-davinci/board-da850-evm.c |4 ++--
 drivers/block/DAC960.c  |4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c|   12 
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c  |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c   |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c |2 +-
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c |3 ++-
 drivers/s390/block/dasd_diag.c  |3 +--
 drivers/scsi/hpsa.c |2 +-
 fs/dlm/plock.c  |3 +--
 fs/ext2/super.c 

Pravin Shelar

[Julia Lawall]

The email address pshe...@nicira.com listed for Pravin Shelar in
MAINTAINERS (OPENVSWITCH section) seems to bounce.

julia

Re: [PATCH v4 net-next 6/6] ila: Route notify (fwd)

[Julia Lawall]

Hello,

Another tab appears to be needed on line 358.

julia

-- Forwarded message --
Date: Sun, 17 Dec 2017 05:42:17 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v4 net-next 6/6] ila: Route notify

CC: kbuild-...@01.org
In-Reply-To: <20171215182800.10248-7-...@quantonium.net>
References: <20171215182800.10248-7-...@quantonium.net>
TO: Tom Herbert <t...@quantonium.net>
CC: da...@davemloft.net
CC: netdev@vger.kernel.org, ro...@cumulusnetworks.com, ro...@quantonium.net, 
Tom Herbert <t...@quantonium.net>

Hi Tom,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Tom-Herbert/net-ILA-notification-mechanism-and-fixes/20171217-041013
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> net/ipv6/ila/ila_lwt.c:358:2-23: code aligned with following code on line 360

# 
https://github.com/0day-ci/linux/commit/bdefe11e33bb3662a60476ea17663189974227a0
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout bdefe11e33bb3662a60476ea17663189974227a0
vim +358 net/ipv6/ila/ila_lwt.c

79ff2fc3 net/ipv6/ila/ila_lwt.c Tom Herbert 2016-10-14  347
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  348  static int 
ila_fill_encap_info(struct sk_buff *skb,
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  349 
   struct lwtunnel_state *lwtstate)
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  350  {
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  351 struct 
ila_params *p = ila_params_lwtunnel(lwtstate);
fddb231e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-11-05  352 struct 
ila_lwt *ilwt = ila_lwt_lwtunnel(lwtstate);
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  353
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  354 if 
(ilwt->xlat) {
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  355 
if (nla_put_u64_64bit(skb, ILA_ATTR_LOCATOR,
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  356 
  (__force u64)p->locator.v64,
f13a82d8 net/ipv6/ila/ila_lwt.c Nicolas Dichtel 2016-04-25  357 
  ILA_ATTR_PAD))
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17 @358 
goto nla_put_failure;
70d5aef4 net/ipv6/ila/ila_lwt.c Tom Herbert 2017-11-05  359
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15 @360 
if (nla_put_u8(skb, ILA_ATTR_CSUM_MODE,
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  361 
   (__force u8)p->csum_mode))
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  362 
goto nla_put_failure;
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  363
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  364 
if (nla_put_u8(skb, ILA_ATTR_IDENT_TYPE,
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  365 
   (__force u8)p->ident_type))
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  366 
goto nla_put_failure;
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  367 }
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  368
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  369 if 
(nla_put_u8(skb, ILA_ATTR_HOOK_TYPE, ilwt->hook_type))
90bfe662 net/ipv6/ila/ila_lwt.c Tom Herbert 2016-04-23  370 
goto nla_put_failure;
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  371
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  372 if 
(ilwt->notify & ILA_NOTIFY_DST)
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  373 
if (nla_put_flag(skb, ILA_ATTR_NOTIFY_DST))
70d5aef4 net/ipv6/ila/ila_lwt.c Tom Herbert 2017-11-05  374 
goto nla_put_failure;
70d5aef4 net/ipv6/ila/ila_lwt.c Tom Herbert 2017-11-05  375
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  376 if 
(ilwt->notify & ILA_NOTIFY_SRC)
bdefe11e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-12-15  377 
if (nla_put_flag(skb, ILA_ATTR_NOTIFY_SRC))
fddb231e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-11-05  378 
goto nla_put_failure;
fddb231e net/ipv6/ila/ila_lwt.c Tom Herbert 2017-11-05  379
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  380 return 
0;
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  381
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  382  
nla_put_failure:
65d7ab8d net/ipv6/ila.c Tom Herbert 2015-08-17  383 return 
-EM

Re: [PATCH v2 3/3] net: macb: change GFP_ATOMIC to GFP_KERNEL

[Julia Lawall]

On Tue, 5 Dec 2017, Julia Cartwright wrote:

> Now that the rx_fs_lock is no longer held across allocation, it's safe
> to use GFP_KERNEL for allocating new entries.
>
> This reverts commit 81da3bf6e3f88 ("net: macb: change GFP_KERNEL to
> GFP_ATOMIC").
>
> Cc: Julia Lawall <julia.law...@lip6.fr>
> Signed-off-by: Julia Cartwright <ju...@ni.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>


> ---
>  drivers/net/ethernet/cadence/macb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 758e8b3042b2..234667eaaa92 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2800,7 +2800,7 @@ static int gem_add_flow_filter(struct net_device 
> *netdev,
>   int ret = -EINVAL;
>   bool added = false;
>
> - newfs = kmalloc(sizeof(*newfs), GFP_ATOMIC);
> + newfs = kmalloc(sizeof(*newfs), GFP_KERNEL);
>   if (newfs == NULL)
>   return -ENOMEM;
>   memcpy(>fs, fs, sizeof(newfs->fs));
> --
> 2.14.2
>
>

[PATCH v2] net: macb: change GFP_KERNEL to GFP_ATOMIC

[Julia Lawall]

Function gem_add_flow_filter called on line 2958 inside lock on line 2949
but uses GFP_KERNEL

Generated by: scripts/coccinelle/locks/call_kern.cocci

Fixes: ae8223de3df5 ("net: macb: Added support for RX filtering")
CC: Rafal Ozieblo <raf...@cadence.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---

v2: Fix some broken email addresses.  No change to the patch.

tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master
head:   fb20eb9d798d2f4c1a75b7fe981d72dfa8d7270d
commit: ae8223de3df5a0ce651d14a50dad31b9cae029f2 [2033/2251] net: macb:
Added support for RX filtering

 macb_main.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2799,7 +2799,7 @@ static int gem_add_flow_filter(struct ne
int ret = -EINVAL;
bool added = false;

-   newfs = kmalloc(sizeof(*newfs), GFP_KERNEL);
+   newfs = kmalloc(sizeof(*newfs), GFP_ATOMIC);
if (newfs == NULL)
return -ENOMEM;
memcpy(>fs, fs, sizeof(newfs->fs));

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Thu, 23 Nov 2017, Greg Kroah-Hartman wrote:

> On Wed, Nov 22, 2017 at 10:20:49PM +0100, Julia Lawall wrote:
> >
> >
> > On Wed, 22 Nov 2017, Joe Perches wrote:
> >
> > > On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > > > There is no need to #define the license of the driver, just put it in
> > > > the MODULE_LICENSE() line directly as a text string.
> > > >
> > > > This allows tools that check that the module license matches the source
> > > > code license to work properly, as there is no need to unwind the
> > > > unneeded dereference.
> > > []
> > > > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> > > []
> > > > @@ -76,7 +76,6 @@
> > > >
> > > >  #define MOD_AUTHOR "Option Wireless"
> > > >  #define MOD_DESCRIPTION"USB High Speed Option 
> > > > driver"
> > > > -#define MOD_LICENSE"GPL"
> > > >
> > > >  #define HSO_MAX_NET_DEVICES10
> > > >  #define HSO__MAX_MTU   2048
> > > > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> > > >
> > > >  MODULE_AUTHOR(MOD_AUTHOR);
> > > >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > > > -MODULE_LICENSE(MOD_LICENSE);
> > > > +MODULE_LICENSE("GPL");
> > >
> > > Probably all of these MODULE_(MOD_) uses could be
> > > simplified as well.
> > >
> > > Perhaps there's utility in a (cocci?) script that looks for
> > > used-once
> > > macro #defines in various types of macros.
> >
> > What about module_version, eg:
> >
> > diff -u -p a/drivers/ata/pata_pdc202xx_old.c
> > b/drivers/ata/pata_pdc202xx_old.c
> > --- a/drivers/ata/pata_pdc202xx_old.c
> > +++ b/drivers/ata/pata_pdc202xx_old.c
> > @@ -21,7 +21,6 @@
> >  #include 
> >
> >  #define DRV_NAME "pata_pdc202xx_old"
> > -#define DRV_VERSION "0.4.3"
> >
> >  static int pdc2026x_cable_detect(struct ata_port *ap)
> >  {
> > @@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
> >  MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
> >  MODULE_LICENSE("GPL");
> >  MODULE_DEVICE_TABLE(pci, pdc202xx);
> > -MODULE_VERSION(DRV_VERSION);
> > +MODULE_VERSION("0.4.3");
>
> I've just deleted MODULE_VERSION() entirely from some subsystems, as
> once the driver is in the kernel source tree, the "version" makes almost
> no sense at all.
>
> But I know some companies love incrementing it (some network and scsi
> drivers specifically), so those might want to keep it around for some
> odd reason.

OK, that seems like a simple soluton.  Thanks.

julia

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Wed, 22 Nov 2017, Joe Perches wrote:

> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > There is no need to #define the license of the driver, just put it in
> > the MODULE_LICENSE() line directly as a text string.
> >
> > This allows tools that check that the module license matches the source
> > code license to work properly, as there is no need to unwind the
> > unneeded dereference.
> []
> > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> []
> > @@ -76,7 +76,6 @@
> >
> >  #define MOD_AUTHOR "Option Wireless"
> >  #define MOD_DESCRIPTION"USB High Speed Option driver"
> > -#define MOD_LICENSE"GPL"
> >
> >  #define HSO_MAX_NET_DEVICES10
> >  #define HSO__MAX_MTU   2048
> > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> >
> >  MODULE_AUTHOR(MOD_AUTHOR);
> >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > -MODULE_LICENSE(MOD_LICENSE);
> > +MODULE_LICENSE("GPL");
>
> Probably all of these MODULE_(MOD_) uses could be
> simplified as well.
>
> Perhaps there's utility in a (cocci?) script that looks for
> used-once
> macro #defines in various types of macros.

What about module_version, eg:

diff -u -p a/drivers/ata/pata_pdc202xx_old.c
b/drivers/ata/pata_pdc202xx_old.c
--- a/drivers/ata/pata_pdc202xx_old.c
+++ b/drivers/ata/pata_pdc202xx_old.c
@@ -21,7 +21,6 @@
 #include 

 #define DRV_NAME "pata_pdc202xx_old"
-#define DRV_VERSION "0.4.3"

 static int pdc2026x_cable_detect(struct ata_port *ap)
 {
@@ -389,4 +388,4 @@ MODULE_AUTHOR("Alan Cox");
 MODULE_DESCRIPTION("low-level driver for Promise 2024x and 20262-20267");
 MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(pci, pdc202xx);
-MODULE_VERSION(DRV_VERSION);
+MODULE_VERSION("0.4.3");

julia

Re: [Outreachy kernel] Re: [PATCH] net: usb: hso.c: remove unneeded DRIVER_LICENSE #define

[Julia Lawall]

On Wed, 22 Nov 2017, Joe Perches wrote:

> On Fri, 2017-11-17 at 15:19 +0100, Greg Kroah-Hartman wrote:
> > There is no need to #define the license of the driver, just put it in
> > the MODULE_LICENSE() line directly as a text string.
> >
> > This allows tools that check that the module license matches the source
> > code license to work properly, as there is no need to unwind the
> > unneeded dereference.
> []
> > diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> []
> > @@ -76,7 +76,6 @@
> >
> >  #define MOD_AUTHOR "Option Wireless"
> >  #define MOD_DESCRIPTION"USB High Speed Option driver"
> > -#define MOD_LICENSE"GPL"
> >
> >  #define HSO_MAX_NET_DEVICES10
> >  #define HSO__MAX_MTU   2048
> > @@ -3288,7 +3287,7 @@ module_exit(hso_exit);
> >
> >  MODULE_AUTHOR(MOD_AUTHOR);
> >  MODULE_DESCRIPTION(MOD_DESCRIPTION);
> > -MODULE_LICENSE(MOD_LICENSE);
> > +MODULE_LICENSE("GPL");
>
> Probably all of these MODULE_(MOD_) uses could be
> simplified as well.
>
> Perhaps there's utility in a (cocci?) script that looks for
> used-once
> macro #defines in various types of macros.

It could be possible.  It's a bit tricky due to ifdefs that Coccinelle
doesn't see and header files, but perhaps in special cases like this there
is not much worry.

julia

>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1511370336.6989.100.camel%40perches.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [PATCH v2 02/15] usb: gadget: make config_item_type structures const

[Julia Lawall]

On Thu, 19 Oct 2017, Laurent Pinchart wrote:

> Hi Christoph,
>
> On Thursday, 19 October 2017 17:06:57 EEST Christoph Hellwig wrote:
> > > Now we have 9 const instances of the config_item_type structure that are
> > > identical, with only the .ct_owner field set. Should they be all merged
> > > into a single structure ?
> >
> > I think that's a good idea.
> >
> > But I'm about to slurp up this whole series into my tree, how about making
> > that an incremental patch?
>
> I'm fine with that.
>
> Bhumika, would you like to submit an incremental patch, or should I do it ?

For various types, there seem to be a few hundred of these, eg:

static const struct hda_pcm_stream alc269_44k_pcm_analog_playback = {
.rates = SNDRV_PCM_RATE_44100, /* fixed rate */
};

static const struct hda_pcm_stream alc269_44k_pcm_analog_capture = {
.rates = SNDRV_PCM_RATE_44100, /* fixed rate */
};

Would it be desirable to remove them?  I guess one would have to check
that there are not any pointer equality checks on these values.  Would it
be useful to put a #define to keep the orignal names?

julia

Re: [PATCH v2 00/15] make structure field, function arguments and structures const

[Julia Lawall]

On Tue, 17 Oct 2017, Greg KH wrote:

> On Mon, Oct 16, 2017 at 05:18:39PM +0200, Bhumika Goyal wrote:
> > Make the ci_type field and some function arguments as const. After this
> > change, make config_item_type structures as const.
> >
> > * Changes in v2- Combine all the followup patches and the constification
> > patches into a series.
>
> Who do you want to take these patches?  If you want, I can take them
> through my driver-core tree, which has done other configfs stuff like
> this in the past.

Christoph Hellwig proposed to take care of it.

julia



>
> thanks,
>
> greg k-h
>

Re: [PATCH] net: phy: DP83822 initial driver submission (fwd)

[Julia Lawall]

DP83822_WOL_CLR_INDICATION appears twice on line 136.  Perhaps this is not
what is wanted.

julia

-- Forwarded message --
Date: Thu, 5 Oct 2017 21:38:28 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH] net: phy: DP83822 initial driver submission

CC: kbuild-...@01.org
In-Reply-To: <20171003155316.12312-1-dmur...@ti.com>
TO: Dan Murphy <dmur...@ti.com>
CC: and...@lunn.ch, f.faine...@gmail.com, netdev@vger.kernel.org, Dan Murphy 
<dmur...@ti.com>
CC: netdev@vger.kernel.org, Dan Murphy <dmur...@ti.com>

Hi Dan,

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Dan-Murphy/net-phy-DP83822-initial-driver-submission/20171005-165547
:: branch date: 5 hours ago
:: commit date: 5 hours ago

>> drivers/net/phy/dp83822.c:136:29-55: duplicated argument to & or |

# 
https://github.com/0day-ci/linux/commit/49190df6a2304f031dc2a6ac63710447db36bc23
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 49190df6a2304f031dc2a6ac63710447db36bc23
vim +136 drivers/net/phy/dp83822.c

49190df6 Dan Murphy 2017-10-03   90
49190df6 Dan Murphy 2017-10-03   91  static int dp83822_set_wol(struct 
phy_device *phydev,
49190df6 Dan Murphy 2017-10-03   92struct 
ethtool_wolinfo *wol)
49190df6 Dan Murphy 2017-10-03   93  {
49190df6 Dan Murphy 2017-10-03   94 struct net_device *ndev = 
phydev->attached_dev;
49190df6 Dan Murphy 2017-10-03   95 u16 value;
49190df6 Dan Murphy 2017-10-03   96 const u8 *mac;
49190df6 Dan Murphy 2017-10-03   97
49190df6 Dan Murphy 2017-10-03   98 if (wol->wolopts & (WAKE_MAGIC | 
WAKE_MAGICSECURE)) {
49190df6 Dan Murphy 2017-10-03   99 mac = (const u8 
*)ndev->dev_addr;
49190df6 Dan Murphy 2017-10-03  100
49190df6 Dan Murphy 2017-10-03  101 if (!is_valid_ether_addr(mac))
49190df6 Dan Murphy 2017-10-03  102 return -EFAULT;
49190df6 Dan Murphy 2017-10-03  103
49190df6 Dan Murphy 2017-10-03  104 /* MAC addresses start with 
byte 5, but stored in mac[0].
49190df6 Dan Murphy 2017-10-03  105  * 822 PHYs store bytes 4|5, 
2|3, 0|1
49190df6 Dan Murphy 2017-10-03  106  */
49190df6 Dan Murphy 2017-10-03  107 phy_write_mmd(phydev, 
DP83822_DEVADDR,
49190df6 Dan Murphy 2017-10-03  108   
MII_DP83822_WOL_DA1, (mac[1] << 8) | mac[0]);
49190df6 Dan Murphy 2017-10-03  109 phy_write_mmd(phydev, 
DP83822_DEVADDR,
49190df6 Dan Murphy 2017-10-03  110   
MII_DP83822_WOL_DA2, (mac[3] << 8) | mac[2]);
49190df6 Dan Murphy 2017-10-03  111 phy_write_mmd(phydev, 
DP83822_DEVADDR, MII_DP83822_WOL_DA3,
49190df6 Dan Murphy 2017-10-03  112   (mac[5] << 8) | 
mac[4]);
49190df6 Dan Murphy 2017-10-03  113
49190df6 Dan Murphy 2017-10-03  114 value = phy_read_mmd(phydev, 
DP83822_DEVADDR,
49190df6 Dan Murphy 2017-10-03  115  
MII_DP83822_WOL_CFG);
49190df6 Dan Murphy 2017-10-03  116 if (wol->wolopts & WAKE_MAGIC)
49190df6 Dan Murphy 2017-10-03  117 value |= 
DP83822_WOL_MAGIC_EN;
49190df6 Dan Murphy 2017-10-03  118 else
49190df6 Dan Murphy 2017-10-03  119 value &= 
~DP83822_WOL_MAGIC_EN;
49190df6 Dan Murphy 2017-10-03  120
49190df6 Dan Murphy 2017-10-03  121 if (wol->wolopts & 
WAKE_MAGICSECURE) {
49190df6 Dan Murphy 2017-10-03  122 value |= 
DP83822_WOL_SECURE_ON;
49190df6 Dan Murphy 2017-10-03  123 phy_write_mmd(phydev, 
DP83822_DEVADDR,
49190df6 Dan Murphy 2017-10-03  124   
MII_DP83822_RXSOP1,
49190df6 Dan Murphy 2017-10-03  125   
(wol->sopass[1] << 8) | wol->sopass[0]);
49190df6 Dan Murphy 2017-10-03  126 phy_write_mmd(phydev, 
DP83822_DEVADDR,
49190df6 Dan Murphy 2017-10-03  127   
MII_DP83822_RXSOP2,
49190df6 Dan Murphy 2017-10-03  128   
(wol->sopass[3] << 8) | wol->sopass[2]);
49190df6 Dan Murphy 2017-10-03  129 phy_write_mmd(phydev, 
DP83822_DEVADDR,
49190df6 Dan Murphy 2017-10-03  130   
MII_DP83822_RXSOP3,
49190df6 Dan Murphy 2017-10-03  131   
(wol->sopass[5] << 8) | wol->sopass[4]);
49190df6 Dan Murphy 2017-10-03  132 } else {
49190df6 Dan Murphy 2017-10-03  133 value &= 
~DP83822_WOL_SECURE_ON;
4919

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Julia Lawall]

On Fri, 22 Sep 2017, Joe Perches wrote:

> On Fri, 2017-09-22 at 12:06 +0200, Julia Lawall wrote:
> >
> > On Fri, 22 Sep 2017, Colin Ian King wrote:
> >
> > > On 22/09/17 11:03, Joe Perches wrote:
> > > > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> > > > >
> > > > > On Thu, 21 Sep 2017, Colin King wrote:
> > > > >
> > > > > > From: Colin Ian King <colin.k...@canonical.com>
> > > > > >
> > > > > > Don't populate const array ac_to_fifo on the stack in an inlined
> > > > > > function, instead make it static.  Makes the object code smaller
> > > > > > by over 800 bytes:
> > > > > >
> > > > > >textdata bss dec hex filename
> > > > > >  159029   331541216  193399   2f377 4965-mac.o
> > > > > >
> > > > > >textdata bss dec hex filename
> > > > > >  158122   332501216  192588   2f04c 4965-mac.o
> > > > > >
> > > > > > (gcc version 7.2.0 x86_64)
> > > > >
> > > > > Gcc 7 must be much more aggressive about inlining than gcc 4.  Here 
> > > > > is the
> > > > > change that I got on this file:
> > > > >
> > > > >  text  data bss dec hex filename
> > > > > -   76346  1494 152   77992   130a8 
> > > > > drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > > +   76298  1494 152   77944   13078 
> > > > > drivers/net/wireless/intel/iwlegacy/4965-mac.o
> > > > > decrease of 48
> > > >
> > > > More likely different CONFIG options.
> > > >
> > > > e.g. allyesconfig vs defconfig with wireless
> > > >
> > >
> > > yup, I used allyesconfig
> >
> > Mine is also allyesconfig.
>
> Odd.
>
> Here is what I get: (Sorry, no gcc-7)
>
> gcc4: gcc-4.9 (Ubuntu 4.9.4-2ubuntu1) 4.9.4
> gcc5: gcc-5 (Ubuntu 5.4.1-8ubuntu1) 5.4.1 20170304
> gcc6: gcc-6 (Ubuntu 6.3.0-12ubuntu2) 6.3.0 20170406
>
> $ size drivers/net/wireless/intel/iwlegacy/4965-mac.o*
>    text      data bss dec hex filename
>  118559      1766 152  120477   1d69d 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.new
>  118623      1766 152  120541   1d6dd 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.allyesconfig.old

My gcc is 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.3)

You get a savings of 64 and I got a savings of 48, but it's not that big a
difference, compared to what the later versions give.

julia

>   51595      1156   0   52751    ce0f 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.new
>   51659      1156   0   52815    ce4f 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc4.defconfig.old
>  141956     29790    1216  172962   2a3a2 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.new
>  142671     29702    1216  173589   2a615 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.allyesconfig.old
>   51733      1156   0   52889    ce99 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.new
>   51813      1156   0   52969    cee9 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc5.defconfig.old
>  152991     29790    1216  183997   2cebd 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.new
>  153722     29702    1216  184640   2d140 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.allyesconfig.old
>   51813      1156   0   52969    cee9 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.new
>   51893      1156   0   53049    cf39 
> drivers/net/wireless/intel/iwlegacy/4965-mac.o.gcc6.defconfig.old
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Julia Lawall]

On Fri, 22 Sep 2017, Colin Ian King wrote:

> On 22/09/17 11:03, Joe Perches wrote:
> > On Fri, 2017-09-22 at 09:23 +0200, Julia Lawall wrote:
> >>
> >> On Thu, 21 Sep 2017, Colin King wrote:
> >>
> >>> From: Colin Ian King <colin.k...@canonical.com>
> >>>
> >>> Don't populate const array ac_to_fifo on the stack in an inlined
> >>> function, instead make it static.  Makes the object code smaller
> >>> by over 800 bytes:
> >>>
> >>>text  data bss dec hex filename
> >>>  159029 331541216  193399   2f377 4965-mac.o
> >>>
> >>>text  data bss dec hex filename
> >>>  158122 332501216  192588   2f04c 4965-mac.o
> >>>
> >>> (gcc version 7.2.0 x86_64)
> >>
> >> Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is the
> >> change that I got on this file:
> >>
> >>  text  data bss dec hex filename
> >> -   76346  1494 152   77992   130a8 
> >> drivers/net/wireless/intel/iwlegacy/4965-mac.o
> >> +   76298  1494 152   77944   13078 
> >> drivers/net/wireless/intel/iwlegacy/4965-mac.o
> >> decrease of 48
> >
> > More likely different CONFIG options.
> >
> > e.g. allyesconfig vs defconfig with wireless
> >
> yup, I used allyesconfig

Mine is also allyesconfig.

julia

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

Re: [PATCH] wireless: iwlegacy: make const array static to shink object code size Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit

[Julia Lawall]

On Thu, 21 Sep 2017, Colin King wrote:

> From: Colin Ian King 
>
> Don't populate const array ac_to_fifo on the stack in an inlined
> function, instead make it static.  Makes the object code smaller
> by over 800 bytes:
>
>text  data bss dec hex filename
>  159029 331541216  193399   2f377 4965-mac.o
>
>text  data bss dec hex filename
>  158122 332501216  192588   2f04c 4965-mac.o
>
> (gcc version 7.2.0 x86_64)

Gcc 7 must be much more aggressive about inlining than gcc 4.  Here is the
change that I got on this file:

 text  data bss dec hex filename
-   76346  1494 152   77992   130a8 
drivers/net/wireless/intel/iwlegacy/4965-mac.o
+   76298  1494 152   77944   13078 
drivers/net/wireless/intel/iwlegacy/4965-mac.o
decrease of 48

julia

>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c 
> b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> index de9b6522c43f..65eba2c24292 100644
> --- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
> @@ -1480,7 +1480,7 @@ il4965_get_ac_from_tid(u16 tid)
>  static inline int
>  il4965_get_fifo_from_tid(u16 tid)
>  {
> - const u8 ac_to_fifo[] = {
> + static const u8 ac_to_fifo[] = {
>   IL_TX_FIFO_VO,
>   IL_TX_FIFO_VI,
>   IL_TX_FIFO_BE,
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [Outreachy kernel] [PATCH] Staging: irda: Use !x instead of NULL comparison

[Julia Lawall]

On Sat, 16 Sep 2017, Srishti Sharma wrote:

> Test for NULL as !x where functions that return NULL on failure
> are used. Done using the following semantic patch by coccinelle.
>
> @ is_null @
> expression E;
> statement S;
> @@
>
> E = (\(kmalloc\|devm_kzalloc\|kmalloc_array\|devm_ioremap\|
> usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\)(...));
>
> (
> if(!E)
>S
> |
> -if(E==NULL)
> +if(!E)
> S
> )
>
> Signed-off-by: Srishti Sharma <srishtis...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>


> ---
>  drivers/staging/irda/net/discovery.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/irda/net/discovery.c 
> b/drivers/staging/irda/net/discovery.c
> index 364d70a..1e54954 100644
> --- a/drivers/staging/irda/net/discovery.c
> +++ b/drivers/staging/irda/net/discovery.c
> @@ -179,7 +179,7 @@ void irlmp_expire_discoveries(hashbin_t *log, __u32 
> saddr, int force)
>   /* Create the client specific buffer */
>   n = HASHBIN_GET_SIZE(log);
>   buffer = kmalloc(n * sizeof(struct 
> irda_device_info), GFP_ATOMIC);
> - if (buffer == NULL) {
> + if (!buffer) {
>   
> spin_unlock_irqrestore(>hb_spinlock, flags);
>   return;
>   }
> @@ -291,7 +291,7 @@ struct irda_device_info *irlmp_copy_discoveries(hashbin_t 
> *log, int *pn,
>   /* Create the client specific buffer */
>   n = HASHBIN_GET_SIZE(log);
>   buffer = kmalloc(n * sizeof(struct 
> irda_device_info), GFP_ATOMIC);
> - if (buffer == NULL) {
> + if (!buffer) {
>   
> spin_unlock_irqrestore(>hb_spinlock, flags);
>   return NULL;
>   }
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1505543667-4670-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment

[Julia Lawall]

On Fri, 15 Sep 2017, Joe Perches wrote:

> On Sat, 2017-09-16 at 02:36 +0530, Srishti Sharma wrote:
> > Parentheses are not needed on the right hand side of assignment
> > statement in most cases. Done using the following semantic
> > patch by coccinelle.
> []
> > @@
> > identifier E,F,G,f;
> > expression e,r;
> > @@
> >
> > (
> > E = (G == F);
> > >
> >
> > E = (e == r);
> > >
> >
> > E =
> > -(
> > ...
> > -)
> > ;
> > )
> []
> > diff --git a/drivers/staging/irda/drivers/mcs7780.c 
> > b/drivers/staging/irda/drivers/mcs7780.c
> []
> > @@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> > if (mcs->new_speed <= 115200) {
> > rval &= ~MCS_FIR;
> >
> > -   rst = (mcs->speed > 115200);
> > +   rst = mcs->speed > 115200;
> > if (rst)
> > mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
> Coccinelle is a good tool, but its output is limited to
> the correctness
> and completeness of its input script.
>
> Please look at the suggested modifications of the script
> and examine the code for other similar uses.
>
> The else if block immediately below this is:
>
>   } else if (mcs->new_speed <= 1152000) {
>   rval &= ~MCS_FIR;
>
>   if ((rst = !(mcs->speed == 576000 || mcs->speed == 11520
> 00)))
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
> which should also be corrected by this patch.

You're concerned about the assignment in the if header?  Because that was
in 1/2.  One could also push the ! under the ||, but I'm not sure that
would be much of an improvement.

julia

>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1505511108.27581.16.camel%40perches.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement

[Julia Lawall]

On Sat, 16 Sep 2017, Srishti Sharma wrote:

> On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.law...@lip6.fr> wrote:
> >
> >
> > On Sat, 16 Sep 2017, Srishti Sharma wrote:
> >
> >> Write assignment statement outside the if statement. Done using
> >> the following semantic patch by coccinelle.
> >>
> >> @@
> >> identifier E;
> >> expression F;
> >> statement S;
> >> @@
> >>
> >> -if((E = F))
> >> +E = F;
> >> +if(E)
> >>   S
> >>
> >> Signed-off-by: Srishti Sharma <srishtis...@gmail.com>
> >
> > Acked-by: Julia Lawall <julia.law...@lip6.fr>
>
> Have sent a patchset for this instead .

Thanks.  Actually, you should have copied the ack.

julia

>
> Regards,
> Srishti
> >
> >> ---
> >> Changes in v2:
> >>  -Semicolon was missing in one of the statements of the
> >>   semantic patch
> >>
> >>  drivers/staging/irda/drivers/irda-usb.c | 4 ++--
> >>  drivers/staging/irda/drivers/mcs7780.c  | 9 ++---
> >>  drivers/staging/irda/net/irqueue.c  | 3 ++-
> >>  3 files changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/staging/irda/drivers/irda-usb.c 
> >> b/drivers/staging/irda/drivers/irda-usb.c
> >> index 723e49b..82bfc05 100644
> >> --- a/drivers/staging/irda/drivers/irda-usb.c
> >> +++ b/drivers/staging/irda/drivers/irda-usb.c
> >> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct 
> >> irda_usb_cb *self)
> >>   urb->transfer_flags = 0;
> >>
> >>   /* Irq disabled -> GFP_ATOMIC */
> >> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> >> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> >> + if (ret)
> >>   net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> >> - }
> >>  }
> >>
> >>  /*--*/
> >> diff --git a/drivers/staging/irda/drivers/mcs7780.c 
> >> b/drivers/staging/irda/drivers/mcs7780.c
> >> index c3f0b25..2b674d5 100644
> >> --- a/drivers/staging/irda/drivers/mcs7780.c
> >> +++ b/drivers/staging/irda/drivers/mcs7780.c
> >> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> >>   if (mcs->new_speed <= 115200) {
> >>   rval &= ~MCS_FIR;
> >>
> >> - if ((rst = (mcs->speed > 115200)))
> >> + rst = (mcs->speed > 115200);
> >> + if (rst)
> >>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
> >>
> >>   } else if (mcs->new_speed <= 1152000) {
> >>   rval &= ~MCS_FIR;
> >>
> >> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> >> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> >> + if (rst)
> >>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
> >>
> >>   } else {
> >>   rval |= MCS_FIR;
> >>
> >> - if ((rst = (mcs->speed != 400)))
> >> + rst = (mcs->speed != 400);
> >> + if (rst)
> >>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
> >>
> >>   }
> >> diff --git a/drivers/staging/irda/net/irqueue.c 
> >> b/drivers/staging/irda/net/irqueue.c
> >> index 160dc89..5aab072 100644
> >> --- a/drivers/staging/irda/net/irqueue.c
> >> +++ b/drivers/staging/irda/net/irqueue.c
> >> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
> >>
> >>   while(*name) {
> >>   h = (h<<4) + *name++;
> >> - if ((g = (h & 0xf000)))
> >> + g = (h & 0xf000);
> >> + if (g)
> >>   h ^=g>>24;
> >>   h &=~g;
> >>   }
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups 
> >> "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an 
> >> email to outreachy-kernel+unsubscr...@googlegroups.com.
> >> To post to this group, send email to outreachy-ker...@googlegroups.com.
> >> To view this discussion on the web visit 
> >> https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1%2BuUkjU%3DZ5bKTJtzUs7sA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement

[Julia Lawall]

On Sat, 16 Sep 2017, Srishti Sharma wrote:

> Write assignment statement outside the if statement. Done using
> the following semantic patch by coccinelle.
>
> @@
> identifier E;
> expression F;
> statement S;
> @@
>
> -if((E = F))
> +E = F;
> +if(E)
>   S
>
> Signed-off-by: Srishti Sharma <srishtis...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>


> ---
> Changes in v2:
>  -Semicolon was missing in one of the statements of the
>   semantic patch
>
>  drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>  drivers/staging/irda/drivers/mcs7780.c  | 9 ++---
>  drivers/staging/irda/net/irqueue.c  | 3 ++-
>  3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/irda/drivers/irda-usb.c 
> b/drivers/staging/irda/drivers/irda-usb.c
> index 723e49b..82bfc05 100644
> --- a/drivers/staging/irda/drivers/irda-usb.c
> +++ b/drivers/staging/irda/drivers/irda-usb.c
> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct 
> irda_usb_cb *self)
>   urb->transfer_flags = 0;
>
>   /* Irq disabled -> GFP_ATOMIC */
> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> + if (ret)
>   net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> - }
>  }
>
>  /*--*/
> diff --git a/drivers/staging/irda/drivers/mcs7780.c 
> b/drivers/staging/irda/drivers/mcs7780.c
> index c3f0b25..2b674d5 100644
> --- a/drivers/staging/irda/drivers/mcs7780.c
> +++ b/drivers/staging/irda/drivers/mcs7780.c
> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>   if (mcs->new_speed <= 115200) {
>   rval &= ~MCS_FIR;
>
> - if ((rst = (mcs->speed > 115200)))
> + rst = (mcs->speed > 115200);
> + if (rst)
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
>   } else if (mcs->new_speed <= 1152000) {
>   rval &= ~MCS_FIR;
>
> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> + if (rst)
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>   } else {
>   rval |= MCS_FIR;
>
> - if ((rst = (mcs->speed != 400)))
> + rst = (mcs->speed != 400);
> + if (rst)
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>   }
> diff --git a/drivers/staging/irda/net/irqueue.c 
> b/drivers/staging/irda/net/irqueue.c
> index 160dc89..5aab072 100644
> --- a/drivers/staging/irda/net/irqueue.c
> +++ b/drivers/staging/irda/net/irqueue.c
> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>
>   while(*name) {
>   h = (h<<4) + *name++;
> - if ((g = (h & 0xf000)))
> + g = (h & 0xf000);
> + if (g)
>   h ^=g>>24;
>   h &=~g;
>   }
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] Staging: irda: Don't use assignment inside if statement

[Julia Lawall]

On Sat, 16 Sep 2017, Srishti Sharma wrote:

> Write assignment statement outside the if statement. Done using
> the following semantic patch by coccinelle.
>
> @@
> identifier E;
> expression F;
> statement S;
> @@
>
> -if((E = F))
> +E = F

The line above would need to end in a ;

This ends up with a lot of assignments with () around the right hand side.
Maybe you could make a series removing them afterwards.

julia

> +if(E)
>   S
>
> Signed-off-by: Srishti Sharma 
> ---
>  drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>  drivers/staging/irda/drivers/mcs7780.c  | 9 ++---
>  drivers/staging/irda/net/irqueue.c  | 3 ++-
>  3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/irda/drivers/irda-usb.c 
> b/drivers/staging/irda/drivers/irda-usb.c
> index 723e49b..82bfc05 100644
> --- a/drivers/staging/irda/drivers/irda-usb.c
> +++ b/drivers/staging/irda/drivers/irda-usb.c
> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct 
> irda_usb_cb *self)
>   urb->transfer_flags = 0;
>
>   /* Irq disabled -> GFP_ATOMIC */
> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> + if (ret)
>   net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> - }
>  }
>
>  /*--*/
> diff --git a/drivers/staging/irda/drivers/mcs7780.c 
> b/drivers/staging/irda/drivers/mcs7780.c
> index c3f0b25..2b674d5 100644
> --- a/drivers/staging/irda/drivers/mcs7780.c
> +++ b/drivers/staging/irda/drivers/mcs7780.c
> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>   if (mcs->new_speed <= 115200) {
>   rval &= ~MCS_FIR;
>
> - if ((rst = (mcs->speed > 115200)))
> + rst = (mcs->speed > 115200);
> + if (rst)
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
>   } else if (mcs->new_speed <= 1152000) {
>   rval &= ~MCS_FIR;
>
> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> + if (rst)
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>   } else {
>   rval |= MCS_FIR;
>
> - if ((rst = (mcs->speed != 400)))
> + rst = (mcs->speed != 400);
> + if (rst)
>   mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
>   }
> diff --git a/drivers/staging/irda/net/irqueue.c 
> b/drivers/staging/irda/net/irqueue.c
> index 160dc89..5aab072 100644
> --- a/drivers/staging/irda/net/irqueue.c
> +++ b/drivers/staging/irda/net/irqueue.c
> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>
>   while(*name) {
>   h = (h<<4) + *name++;
> - if ((g = (h & 0xf000)))
> + g = (h & 0xf000);
> + if (g)
>   h ^=g>>24;
>   h &=~g;
>   }
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1505504036-21807-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: irda: Remove typedef struct

[Julia Lawall]

On Wed, 13 Sep 2017, Haneen Mohammed wrote:

> This patch remove typedef from a structure with all its ocurrences
> since using typedefs for structures is discouraged.
> Issue found using Coccinelle:
>
> @r1@
> type T;
> @@
>
> typedef struct { ... } T;
>
> @script:python c1@
> T2;
> T << r1.T;
> @@
> if T[-2:] =="_t" or T[-2:] == "_T":
>   coccinelle.T2 = T[:-2];
> else:
>   coccinelle.T2 = T;
>
> print T, coccinelle.T2
>
> @r2@
> type r1.T;
> identifier c1.T2;
> @@
> -typedef
> struct
> + T2
> { ... }
> -T
> ;
>
> @r3@
> type r1.T;
> identifier c1.T2;
> @@
> -T
> +struct T2
>
> Signed-off-by: Haneen Mohammed <hamohammed...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>

> ---
>  drivers/staging/irda/include/net/irda/qos.h | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/irda/include/net/irda/qos.h 
> b/drivers/staging/irda/include/net/irda/qos.h
> index 05a5a24..a0315b5 100644
> --- a/drivers/staging/irda/include/net/irda/qos.h
> +++ b/drivers/staging/irda/include/net/irda/qos.h
> @@ -58,23 +58,23 @@
>  #define IR_1600 0x02
>
>  /* Quality of Service information */
> -typedef struct {
> +struct qos_value {
>   __u32 value;
>   __u16 bits; /* LSB is first byte, MSB is second byte */
> -} qos_value_t;
> +};
>
>  struct qos_info {
>   magic_t magic;
>
> - qos_value_t baud_rate;   /* IR_11520O | ... */
> - qos_value_t max_turn_time;
> - qos_value_t data_size;
> - qos_value_t window_size;
> - qos_value_t additional_bofs;
> - qos_value_t min_turn_time;
> - qos_value_t link_disc_time;
> + struct qos_value baud_rate;   /* IR_11520O | ... */
> + struct qos_value max_turn_time;
> + struct qos_value data_size;
> + struct qos_value window_size;
> + struct qos_value additional_bofs;
> + struct qos_value min_turn_time;
> + struct qos_value link_disc_time;
>
> - qos_value_t power;
> + struct qos_value power;
>  };
>
>  extern int sysctl_max_baud_rate;
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170914045538.GA24121%40Haneen.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [PATCH] Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile (fwd)

[Julia Lawall]

It is intentional that the code on lines 122 and 124 is the same?

julia

-- Forwarded message --
Date: Thu, 17 Aug 2017 09:18:50 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH]
Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile

Hi mohamedalrshah,

[auto build test WARNING on net/master]
[also build test WARNING on v4.13-rc5 next-20170816]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/mohamedalrshah/Adding-Agile-SD-TCP-module-and-modifying-Kconfig-and-makefile/20170817-055643
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> net/ipv4/tcp_agilesd.c:121:1-3: WARNING: possible condition with no effect 
>> (if == else)

# 
https://github.com/0day-ci/linux/commit/839e8cb5e5f068e0310442909f9a89753a218c59
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 839e8cb5e5f068e0310442909f9a89753a218c59
vim +121 net/ipv4/tcp_agilesd.c

839e8cb5 mohamedalrshah 2017-08-15  111
839e8cb5 mohamedalrshah 2017-08-15  112  /* This function is called when the 
TCP flow detects a loss.
839e8cb5 mohamedalrshah 2017-08-15  113   * It returns the slow start threshold 
of a flow, after a packet loss is detected. */
839e8cb5 mohamedalrshah 2017-08-15  114  static u32 
agilesdtcp_recalc_ssthresh(struct sock *sk)
839e8cb5 mohamedalrshah 2017-08-15  115  {
839e8cb5 mohamedalrshah 2017-08-15  116 const struct tcp_sock *tp = 
tcp_sk(sk);
839e8cb5 mohamedalrshah 2017-08-15  117 struct agilesdtcp *ca = 
inet_csk_ca(sk);
839e8cb5 mohamedalrshah 2017-08-15  118
839e8cb5 mohamedalrshah 2017-08-15  119 ca->loss_cwnd = tp->snd_cwnd;
839e8cb5 mohamedalrshah 2017-08-15  120
839e8cb5 mohamedalrshah 2017-08-15 @121 if (ca->agilesd_tcp_status == 
CA)
839e8cb5 mohamedalrshah 2017-08-15  122 ca->degraded_loss_cwnd 
= max((tp->snd_cwnd * beta) / SCALE, 2U);
839e8cb5 mohamedalrshah 2017-08-15  123 else
839e8cb5 mohamedalrshah 2017-08-15  124 ca->degraded_loss_cwnd 
= max((tp->snd_cwnd * beta) / SCALE, 2U);
839e8cb5 mohamedalrshah 2017-08-15  125
839e8cb5 mohamedalrshah 2017-08-15  126 ca->frac_tracer = 0;
839e8cb5 mohamedalrshah 2017-08-15  127
839e8cb5 mohamedalrshah 2017-08-15  128 return ca->degraded_loss_cwnd;
839e8cb5 mohamedalrshah 2017-08-15  129  }
839e8cb5 mohamedalrshah 2017-08-15  130

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH net-next 2/3] Add LAN743X to Kconfig and Makefile (fwd)

[Julia Lawall]

I don't think netdev_priv can return NULL, so lines 6641 to 6646 could
just be dropped.

julia

-- Forwarded message --
Date: Mon, 14 Aug 2017 18:14:12 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH net-next 2/3] Add LAN743X to Kconfig and Makefile

CC: kbuild-...@01.org
In-Reply-To: 
<90a7e81ae28bae4cbddb3b35f187d264406f6...@chn-sv-exmx02.mchp-main.com>
TO: bryan.whiteh...@microchip.com
CC: netdev@vger.kernel.org, da...@davemloft.net
CC: unglinuxdri...@microchip.com

Hi Bryan,

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Bryan-Whitehead-microchip-com/Add-LAN743X-driver/20170814-141247
:: branch date: 4 hours ago
:: commit date: 4 hours ago

>> drivers/net/ethernet/microchip/lan743x.c:6642:39-45: ERROR: adapter is NULL 
>> but dereferenced.

# 
https://github.com/0day-ci/linux/commit/dabee19fb26fee5184b2b304b12080be0f78a3f2
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout dabee19fb26fee5184b2b304b12080be0f78a3f2
vim +6642 drivers/net/ethernet/microchip/lan743x.c

a33f5600 Bryan Whitehead 2017-08-11  6609
a33f5600 Bryan Whitehead 2017-08-11  6610  /* lan743x_pcidev_probe - Device 
Initialization Routine
a33f5600 Bryan Whitehead 2017-08-11  6611   * @pdev: PCI device information 
struct
a33f5600 Bryan Whitehead 2017-08-11  6612   * @id: entry in lan743x_pci_tbl
a33f5600 Bryan Whitehead 2017-08-11  6613   *
a33f5600 Bryan Whitehead 2017-08-11  6614   * Returns 0 on success, negative on 
failure
a33f5600 Bryan Whitehead 2017-08-11  6615   *
a33f5600 Bryan Whitehead 2017-08-11  6616   * initializes an adapter identified 
by a pci_dev structure.
a33f5600 Bryan Whitehead 2017-08-11  6617   * The OS initialization, 
configuring of the adapter private structure,
a33f5600 Bryan Whitehead 2017-08-11  6618   * and a hardware reset occur.
a33f5600 Bryan Whitehead 2017-08-11  6619   **/
a33f5600 Bryan Whitehead 2017-08-11  6620  static int 
lan743x_pcidev_probe(struct pci_dev *pdev,
a33f5600 Bryan Whitehead 2017-08-11  6621   const 
struct pci_device_id *id)
a33f5600 Bryan Whitehead 2017-08-11  6622  {
a33f5600 Bryan Whitehead 2017-08-11  6623   struct net_device *netdev = 
NULL;
a33f5600 Bryan Whitehead 2017-08-11  6624   struct lan743x_adapter *adapter 
= NULL;
a33f5600 Bryan Whitehead 2017-08-11  6625   int ret = -ENODEV;
a33f5600 Bryan Whitehead 2017-08-11  6626
a33f5600 Bryan Whitehead 2017-08-11  6627   NETIF_ASSERT(adapter, probe, 
adapter->netdev, pdev);
a33f5600 Bryan Whitehead 2017-08-11  6628
a33f5600 Bryan Whitehead 2017-08-11  6629   netdev = 
alloc_etherdev(sizeof(struct lan743x_adapter));
a33f5600 Bryan Whitehead 2017-08-11  6630   if (!netdev) {
a33f5600 Bryan Whitehead 2017-08-11  6631   NETIF_ERROR(adapter, 
probe, adapter->netdev,
a33f5600 Bryan Whitehead 2017-08-11  6632   
"alloc_etherdev returned NULL");
a33f5600 Bryan Whitehead 2017-08-11  6633   ret = -ENOMEM;
a33f5600 Bryan Whitehead 2017-08-11  6634   goto clean_up;
a33f5600 Bryan Whitehead 2017-08-11  6635   }
a33f5600 Bryan Whitehead 2017-08-11  6636
a33f5600 Bryan Whitehead 2017-08-11  6637   strncpy(netdev->name, 
pci_name(pdev), sizeof(netdev->name) - 1);
a33f5600 Bryan Whitehead 2017-08-11  6638   SET_NETDEV_DEV(netdev, 
>dev);
a33f5600 Bryan Whitehead 2017-08-11  6639   pci_set_drvdata(pdev, netdev);
a33f5600 Bryan Whitehead 2017-08-11  6640   adapter = netdev_priv(netdev);
a33f5600 Bryan Whitehead 2017-08-11  6641   if (!adapter) {
a33f5600 Bryan Whitehead 2017-08-11 @6642   NETIF_ERROR(adapter, 
probe, adapter->netdev,
a33f5600 Bryan Whitehead 2017-08-11  6643   
"netdev_priv returned NULL");
a33f5600 Bryan Whitehead 2017-08-11  6644   ret = -ENOMEM;
a33f5600 Bryan Whitehead 2017-08-11  6645   goto clean_up;
a33f5600 Bryan Whitehead 2017-08-11  6646   }
a33f5600 Bryan Whitehead 2017-08-11  6647   memset(adapter, 0, 
sizeof(struct lan743x_adapter));
a33f5600 Bryan Whitehead 2017-08-11  6648   adapter->netdev = netdev;
a33f5600 Bryan Whitehead 2017-08-11  6649   adapter->init_flags = 0;
a33f5600 Bryan Whitehead 2017-08-11  6650   adapter->open_flags = 0;
a33f5600 Bryan Whitehead 2017-08-11  6651   adapter->msg_enable = 
msg_enable;
a33f5600 Bryan Whitehead 2017-08-11  6652   netdev->max_mtu = 
LAN743X_MAX_FRAME_SIZE;
a33f5600 Bryan Whitehead 2017-08-11  6653
a33f5600 Bryan Whitehead 2017-08-11  6654   ret = lan743x_pci_init(adapter, 
pdev);
a33f5600 Bryan Whitehead 2017-08-11  6655   if (ret) {
a33f5600 Bryan Whitehead 2017-08-11  6656   NETIF_ERROR(adapter, 
probe, adapter->netdev,
a33f5600 Bry

[PATCH] X25: constify null_x25_address

[Julia Lawall]

null_x25_address is only used to access the string it contains, so it can
be const.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/x25/af_x25.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 5a1a98d..ac09593 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -74,7 +74,7 @@
 
 static const struct proto_ops x25_proto_ops;
 
-static struct x25_address null_x25_address = {"   "};
+static const struct x25_address null_x25_address = {"   "};
 
 #ifdef CONFIG_COMPAT
 struct compat_x25_subscrip_struct {

[PATCH 1/2] Revert "ipv6: constify inet6_protocol structures"

[Julia Lawall]

This reverts commit 3a3a4e3054137c5ff5d4d306ec834f6d25d7f95b.

inet6_add_protocol and inet6_del_protocol include casts that remove the
effect of the const annotation on their parameter, leading to possible
runtime crashes.

Reported-by: Eric Dumazet <eric.duma...@gmail.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

 net/ipv6/ip6_gre.c  |2 +-
 net/ipv6/tcp_ipv6.c |2 +-
 net/ipv6/udp.c  |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 33865d6..67ff2aa 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1080,7 +1080,7 @@ static void ip6gre_fb_tunnel_init(struct net_device *dev)
 }
 
 
-static const struct inet6_protocol ip6gre_protocol = {
+static struct inet6_protocol ip6gre_protocol __read_mostly = {
.handler = gre_rcv,
.err_handler = ip6gre_err,
.flags   = INET6_PROTO_NOPOLICY|INET6_PROTO_FINAL,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 39ee8e7..ced5dcf 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1944,7 +1944,7 @@ struct proto tcpv6_prot = {
.diag_destroy   = tcp_abort,
 };
 
-static const struct inet6_protocol tcpv6_protocol = {
+static struct inet6_protocol tcpv6_protocol = {
.early_demux=   tcp_v6_early_demux,
.early_demux_handler =  tcp_v6_early_demux,
.handler=   tcp_v6_rcv,
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 7e6d7f5..98fe456 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1457,7 +1457,7 @@ int compat_udpv6_getsockopt(struct sock *sk, int level, 
int optname,
 }
 #endif
 
-static const struct inet6_protocol udpv6_protocol = {
+static struct inet6_protocol udpv6_protocol = {
.early_demux=   udp_v6_early_demux,
.early_demux_handler =  udp_v6_early_demux,
.handler=   udpv6_rcv,

[PATCH 2/2] Revert "l2tp: constify inet6_protocol structures"

[Julia Lawall]

This reverts commit d04916a48ad4a3db892b664fa9c3a2a693c378ad.

inet6_add_protocol and inet6_del_protocol include casts that remove the
effect of the const annotation on their parameter, leading to possible
runtime crashes.

Reported-by: Eric Dumazet <eric.duma...@gmail.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

 net/l2tp/l2tp_ip6.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index d2efcd9..88b397c 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -788,7 +788,7 @@ static int l2tp_ip6_recvmsg(struct sock *sk, struct msghdr 
*msg, size_t len,
.ops= _ip6_ops,
 };
 
-static const struct inet6_protocol l2tp_ip6_protocol = {
+static struct inet6_protocol l2tp_ip6_protocol __read_mostly = {
.handler= l2tp_ip6_recv,
 };
 

[PATCH 0/2] Revert "ipv6: constify inet6_protocol structures"

[Julia Lawall]

inet6_add_protocol and inet6_del_protocol include casts that remove the
effect of the const annotation on their parameter, leading to possible
runtime crashes.

Re: [PATCH 1/2] ipv6: constify inet6_protocol structures

[Julia Lawall]

On Tue, 1 Aug 2017, David Ahern wrote:

> On 7/31/17 11:59 PM, Julia Lawall wrote:
> >> This change breaks the kernel if one of these sysctls are changed:
> >> tcp_early_demux, udp_early_demux
> >
> > The other patch in the series has the same problem and should be dropped
> > too.
> >
> > julia
>
> Julia: are you going to send a revert patch? Right now I have to do that
> manually before launching test scripts.

Sorry, I didn't know it was applied.  I can send it.

julia

[PATCH 1/1 v3 nf-next] netfilter: constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.

This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l3/4proto structures const
subsequently.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

v3:

Rebased against nf-next.  Some functions, such as
nf_ct_l3proto_pernet_register, are no longer defined, so they are no longer
updated.

 include/net/netfilter/nf_conntrack_l4proto.h |   14 +++---
 include/net/netfilter/nf_conntrack_timeout.h |2 +-
 net/netfilter/nf_conntrack_core.c|8 
 net/netfilter/nf_conntrack_netlink.c |6 +++---
 net/netfilter/nf_conntrack_proto.c   |   24 
 net/netfilter/nfnetlink_cttimeout.c  |5 +++--
 6 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l4proto.h 
b/include/net/netfilter/nf_conntrack_l4proto.h
index 7032e04..b6e27ca 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -125,23 +125,23 @@ struct nf_conntrack_l4proto 
*__nf_ct_l4proto_find(u_int16_t l3proto,
 
 struct nf_conntrack_l4proto *nf_ct_l4proto_find_get(u_int16_t l3proto,
u_int8_t l4proto);
-void nf_ct_l4proto_put(struct nf_conntrack_l4proto *p);
+void nf_ct_l4proto_put(const struct nf_conntrack_l4proto *p);
 
 /* Protocol pernet registration. */
 int nf_ct_l4proto_pernet_register_one(struct net *net,
- struct nf_conntrack_l4proto *proto);
+   const struct nf_conntrack_l4proto *proto);
 void nf_ct_l4proto_pernet_unregister_one(struct net *net,
-struct nf_conntrack_l4proto *proto);
+   const struct nf_conntrack_l4proto *proto);
 int nf_ct_l4proto_pernet_register(struct net *net,
- struct nf_conntrack_l4proto *proto[],
+ struct nf_conntrack_l4proto *const proto[],
  unsigned int num_proto);
 void nf_ct_l4proto_pernet_unregister(struct net *net,
-struct nf_conntrack_l4proto *proto[],
-unsigned int num_proto);
+   struct nf_conntrack_l4proto *const proto[],
+   unsigned int num_proto);
 
 /* Protocol global registration. */
 int nf_ct_l4proto_register_one(struct nf_conntrack_l4proto *proto);
-void nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *proto);
+void nf_ct_l4proto_unregister_one(const struct nf_conntrack_l4proto *proto);
 int nf_ct_l4proto_register(struct nf_conntrack_l4proto *proto[],
   unsigned int num_proto);
 void nf_ct_l4proto_unregister(struct nf_conntrack_l4proto *proto[],
diff --git a/include/net/netfilter/nf_conntrack_timeout.h 
b/include/net/netfilter/nf_conntrack_timeout.h
index d40b893..b222957 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -68,7 +68,7 @@ struct nf_conn_timeout *nf_ct_timeout_ext_add(struct nf_conn 
*ct,
 
 static inline unsigned int *
 nf_ct_timeout_lookup(struct net *net, struct nf_conn *ct,
-struct nf_conntrack_l4proto *l4proto)
+const struct nf_conntrack_l4proto *l4proto)
 {
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
struct nf_conn_timeout *timeout_ext;
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index 2bc4991..f2f00ea 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1176,8 +1176,8 @@ void nf_conntrack_free(struct nf_conn *ct)
 static noinline struct nf_conntrack_tuple_hash *
 init_conntrack(struct net *net, struct nf_conn *tmpl,
   const struct nf_conntrack_tuple *tuple,
-  struct nf_conntrack_l3proto *l3proto,
-  struct nf_conntrack_l4proto *l4proto,
+  const struct nf_conntrack_l3proto *l3proto,
+  const struct nf_conntrack_l4proto *l4proto,
   struct sk_buff *skb,
   unsigned int dataoff, u32 hash)
 {
@@ -1288,8 +1288,8 @@ void nf_conntrack_free(struct nf_conn *ct)
  unsigned int dataoff,
  u_int16_t l3num,
  u_int8_t protonum,
- struct nf_conntrack_l3proto *l3proto,
- struct nf_conntrack_l4proto *l4proto)
+ const struct nf_conntrack_l3proto *l3proto,
+ const struct nf_conntrack_l4proto *l4proto)
 {
const struct nf_conntrack_zone *zone;
struct nf_conntrack_tuple tuple;
diff --git a/net/net

[PATCH 0/1 v3 nf-next] constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.

This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l4proto structures const
subsequently.

Done with the help of Coccinelle.  The following semantic patch shows
the nf_conntrack_l3proto case.

// 
virtual update_results
virtual after_start

@initialize:ocaml@
@@

let unsafe = Hashtbl.create 101

let is_unsafe f = Hashtbl.mem unsafe f

let changed = ref false


(* The next three rules relate to the fact that we do not know the type of
void * variables.  Fortunately this is only neede on the first iteration,
but it still means that the whole kernel will end up being considered. *)

@has depends on !after_start && !update_results@
identifier f,x;
position p;
@@

f@p(...,struct nf_conntrack_l3proto *x,...) { ... }

@hasa depends on !after_start@
identifier f,x;
position p;
@@

f@p(...,struct nf_conntrack_l3proto *x[],...) { ... }

@others depends on !after_start && !update_results@
position p != {has.p,hasa.p};
identifier f,x;
@@

f@p(...,void *x,...) { ... }

@script:ocaml@
f << others.f;
@@

changed := true;
Hashtbl.add unsafe f ()


@fpb depends on !update_results disable optional_qualifier, drop_cast exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
type T;
@@

f(...,struct nf_conntrack_l3proto *x,...)
{
...
(
  return x;
|
  (<+...x...+>) aop e
|
  e aop x
|
  (T)x
|
  &(<+...x...+>)
|
  bad(...,x,...)
|
  fp(...,x,...)
|
  (<+...e->fld...+>)(...,x,...)
)
...when any
 }

@script:ocaml@
f << fpb.f;
@@

changed := true;
Printf.eprintf "%s is unsafe\n" f;
Hashtbl.add unsafe f ()

@fpba depends on !update_results disable optional_qualifier, drop_cast exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
type T;
@@

f(...,struct nf_conntrack_l3proto *x[],...)
{
...
(
  return \(x\|x[...]\);
|
  (<+...x...+>) aop e
|
  e aop \(x\|x[...]\)
|
  (T)\(x\|x[...]\)
|
  &(<+...x...+>)
|
  bad(...,\(x\|x[...]\),...)
|
  fp(...,\(x\|x[...]\),...)
|
  (<+...e->fld...+>)(...,\(x\|x[...]\),...)
)
... when any
 }

@script:ocaml@
f << fpba.f;
@@

changed := true;
Printf.eprintf "%s is unsafe\n" f;
Hashtbl.add unsafe f ()

@finalize:ocaml depends on !update_results@
tbls << merge.unsafe;
c << merge.changed;
@@

List.iter
(fun t ->
  Hashtbl.iter
(fun k v ->
  if not (Hashtbl.mem unsafe k) then Hashtbl.add unsafe k ()) t)
tbls;
changed := false;
let changed = List.exists (fun x -> !x) c in
let it = new iteration() in
it#add_virtual_rule After_start;
(if not changed
then it#add_virtual_rule Update_results);
it#register()

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
@@

f(...,
+ const
  struct nf_conntrack_l3proto *x,...) { ... }

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
type T;
@@

T f(...,
+ const
  struct nf_conntrack_l3proto *x,...);

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
@@

f(...,
  struct nf_conntrack_l3proto *
+ const
  x[],...) { ... }

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
type T;
@@

T f(...,
  struct nf_conntrack_l3proto *
+ const
  x[],...);
// 

---

v3:

Rebased against nf-next.  Some functions, such as
nf_ct_l3proto_pernet_register, are no longer defined, so they are no longer
updated.

 include/net/netfilter/nf_conntrack_l4proto.h |   14 +++---
 include/net/netfilter/nf_conntrack_timeout.h |2 +-
 net/netfilter/nf_conntrack_core.c|8 
 net/netfilter/nf_conntrack_netlink.c |6 +++---
 net/netfilter/nf_conntrack_proto.c   |   24 
 net/netfilter/nfnetlink_cttimeout.c  |5 +++--
 6 files changed, 30 insertions(+), 29 deletions(-)

Re: [PATCH 1/2] ipv6: constify inet6_protocol structures

[Julia Lawall]

On Mon, 31 Jul 2017, Eric Dumazet wrote:

> On Fri, 2017-07-28 at 22:18 +0200, Julia Lawall wrote:
> > The inet6_protocol structure is only passed as the first argument to
> > inet6_add_protocol or inet6_del_protocol, both of which are declared as
> > const.  Thus the inet6_protocol structure itself can be const.
> >
> > Also drop __read_mostly where present on the newly const structures.
> >
> > Done with the help of Coccinelle.
> >
> > Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
> >
> > ---
> >  net/ipv6/ip6_gre.c  |2 +-
> >  net/ipv6/tcp_ipv6.c |2 +-
> >  net/ipv6/udp.c  |2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> > index 67ff2aa..33865d6 100644
> > --- a/net/ipv6/ip6_gre.c
> > +++ b/net/ipv6/ip6_gre.c
> > @@ -1080,7 +1080,7 @@ static void ip6gre_fb_tunnel_init(struct net_device 
> > *dev)
> >  }
> >
> >
> > -static struct inet6_protocol ip6gre_protocol __read_mostly = {
> > +static const struct inet6_protocol ip6gre_protocol = {
> > .handler = gre_rcv,
> > .err_handler = ip6gre_err,
> > .flags   = INET6_PROTO_NOPOLICY|INET6_PROTO_FINAL,
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 90a3257..2968a33 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1945,7 +1945,7 @@ struct proto tcpv6_prot = {
> > .diag_destroy   = tcp_abort,
> >  };
> >
> > -static struct inet6_protocol tcpv6_protocol = {
> > +static const struct inet6_protocol tcpv6_protocol = {
> > .early_demux=   tcp_v6_early_demux,
> > .early_demux_handler =  tcp_v6_early_demux,
> > .handler=   tcp_v6_rcv,
> > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> > index 4a3e656..5f8b8d7 100644
> > --- a/net/ipv6/udp.c
> > +++ b/net/ipv6/udp.c
> > @@ -1448,7 +1448,7 @@ int compat_udpv6_getsockopt(struct sock *sk, int 
> > level, int optname,
> >  }
> >  #endif
> >
> > -static struct inet6_protocol udpv6_protocol = {
> > +static const struct inet6_protocol udpv6_protocol = {
> > .early_demux=   udp_v6_early_demux,
> > .early_demux_handler =  udp_v6_early_demux,
> > .handler=   udpv6_rcv,
> >
>
> This change breaks the kernel if one of these sysctls are changed:
> tcp_early_demux, udp_early_demux

The other patch in the series has the same problem and should be dropped
too.

julia


>
> Check commit dddb64bcb346 ("net: Add sysctl to toggle early demux for
> tcp and udp") why at least 2 structures were no longer const.
>
> (none):~# echo 0 >/proc/sys/net/ipv4/udp_early_demux
> [  101.746108] BUG: unable to handle kernel paging request at b98cf5c0
> [  101.753093] IP: proc_udp_early_demux+0x46/0x60
> [  101.757565] PGD 13f540a067
> [  101.757565] P4D 13f540a067
> [  101.760372] PUD 13f540b063
> [  101.763171] PMD 8013f50001e1
> [  101.765960]
> [  101.770790] Oops: 0003 [#1] SMP
> [  101.774376] gsmi: Log Shutdown Reason 0x03
> [  101.778473] Modules linked in: w1_therm wire cdc_acm ehci_pci ehci_hcd 
> mlx4_en ib_uverbs mlx4_ib ib_core mlx4_core
> [  101.788890] CPU: 3 PID: 8819 Comm: bash Not tainted 4.13.0-smp-DEV #290
> [  101.795549] Hardware name: Intel RML,PCH/Iota_QC_19, BIOS 2.40.0 06/22/2016
> [  101.802517] task: 8e733b9c4140 task.stack: a43548cd
> [  101.808445] RIP: 0010:proc_udp_early_demux+0x46/0x60
> [  101.813445] RSP: 0018:a43548cd3e60 EFLAGS: 00010246
> [  101.818676] RAX:  RBX: 0001 RCX: 
> 
> [  101.825816] RDX: b98cf5c0 RSI:  RDI: 
> 8e733f400100
> [  101.832957] RBP: a43548cd3e68 R08:  R09: 
> 0001
> [  101.840096] R10: 0008 R11: f000 R12: 
> 0001
> [  101.847260] R13: ffea R14: 0002 R15: 
> b9d00380
> [  101.854434] FS:  7f7e1f34a700() GS:8e733f8c() 
> knlGS:
> [  101.862518] CS:  0010 DS:  ES:  CR0: 80050033
> [  101.868263] CR2: b98cf5c0 CR3: 00100f0c8000 CR4: 
> 001406e0
> [  101.875439] Call Trace:
> [  101.877888]  proc_sys_call_handler+0xf3/0x190
> [  101.882260]  proc_sys_write+0x14/0x20
> [  101.885944]  vfs_write+0xc8/0x1e0
> [  101.889261]  SyS_write+0x48/0xa0
> [  101.892520]  entry_SYSCALL_64_fastpath+0x13/0x94
> [  101.897137] RIP: 0033:0x7f7e1ebd64a0
> [  101.900707] RSP: 002b:7ffcbb851c58 EFLAGS: 024

Re: [PATCH 1/2] ipv6: constify inet6_protocol structures

[Julia Lawall]

On Mon, 31 Jul 2017, Eric Dumazet wrote:

> On Fri, 2017-07-28 at 22:18 +0200, Julia Lawall wrote:
> > The inet6_protocol structure is only passed as the first argument to
> > inet6_add_protocol or inet6_del_protocol, both of which are declared as
> > const.  Thus the inet6_protocol structure itself can be const.
> >
> > Also drop __read_mostly where present on the newly const structures.
> >
> > Done with the help of Coccinelle.
> >
> > Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
> >
> > ---
> >  net/ipv6/ip6_gre.c  |2 +-
> >  net/ipv6/tcp_ipv6.c |2 +-
> >  net/ipv6/udp.c  |2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> > index 67ff2aa..33865d6 100644
> > --- a/net/ipv6/ip6_gre.c
> > +++ b/net/ipv6/ip6_gre.c
> > @@ -1080,7 +1080,7 @@ static void ip6gre_fb_tunnel_init(struct net_device 
> > *dev)
> >  }
> >
> >
> > -static struct inet6_protocol ip6gre_protocol __read_mostly = {
> > +static const struct inet6_protocol ip6gre_protocol = {
> > .handler = gre_rcv,
> > .err_handler = ip6gre_err,
> > .flags   = INET6_PROTO_NOPOLICY|INET6_PROTO_FINAL,
> > diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> > index 90a3257..2968a33 100644
> > --- a/net/ipv6/tcp_ipv6.c
> > +++ b/net/ipv6/tcp_ipv6.c
> > @@ -1945,7 +1945,7 @@ struct proto tcpv6_prot = {
> > .diag_destroy   = tcp_abort,
> >  };
> >
> > -static struct inet6_protocol tcpv6_protocol = {
> > +static const struct inet6_protocol tcpv6_protocol = {
> > .early_demux=   tcp_v6_early_demux,
> > .early_demux_handler =  tcp_v6_early_demux,
> > .handler=   tcp_v6_rcv,
> > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> > index 4a3e656..5f8b8d7 100644
> > --- a/net/ipv6/udp.c
> > +++ b/net/ipv6/udp.c
> > @@ -1448,7 +1448,7 @@ int compat_udpv6_getsockopt(struct sock *sk, int 
> > level, int optname,
> >  }
> >  #endif
> >
> > -static struct inet6_protocol udpv6_protocol = {
> > +static const struct inet6_protocol udpv6_protocol = {
> > .early_demux=   udp_v6_early_demux,
> > .early_demux_handler =  udp_v6_early_demux,
> > .handler=   udpv6_rcv,
> >
>
> This change breaks the kernel if one of these sysctls are changed:
> tcp_early_demux, udp_early_demux

Thanks for the feedback.  The structure is passed to:

int inet6_add_protocol(const struct inet6_protocol *prot, unsigned char
protocol)
{
return !cmpxchg((const struct inet6_protocol **)_protos[protocol],
NULL, prot) ? 0 : -1;
}

I guess that things go wrong because inet6_protos does not really contain
const structures, as hinted by the cast?  And the purpose of declaring a
parameter as const when it is not is to allow things that actually are
able to be const to be passed in here?

thanks,
julia

>
> Check commit dddb64bcb346 ("net: Add sysctl to toggle early demux for
> tcp and udp") why at least 2 structures were no longer const.
>
> (none):~# echo 0 >/proc/sys/net/ipv4/udp_early_demux
> [  101.746108] BUG: unable to handle kernel paging request at b98cf5c0
> [  101.753093] IP: proc_udp_early_demux+0x46/0x60
> [  101.757565] PGD 13f540a067
> [  101.757565] P4D 13f540a067
> [  101.760372] PUD 13f540b063
> [  101.763171] PMD 8013f50001e1
> [  101.765960]
> [  101.770790] Oops: 0003 [#1] SMP
> [  101.774376] gsmi: Log Shutdown Reason 0x03
> [  101.778473] Modules linked in: w1_therm wire cdc_acm ehci_pci ehci_hcd 
> mlx4_en ib_uverbs mlx4_ib ib_core mlx4_core
> [  101.788890] CPU: 3 PID: 8819 Comm: bash Not tainted 4.13.0-smp-DEV #290
> [  101.795549] Hardware name: Intel RML,PCH/Iota_QC_19, BIOS 2.40.0 06/22/2016
> [  101.802517] task: 8e733b9c4140 task.stack: a43548cd
> [  101.808445] RIP: 0010:proc_udp_early_demux+0x46/0x60
> [  101.813445] RSP: 0018:a43548cd3e60 EFLAGS: 00010246
> [  101.818676] RAX:  RBX: 0001 RCX: 
> 
> [  101.825816] RDX: b98cf5c0 RSI:  RDI: 
> 8e733f400100
> [  101.832957] RBP: a43548cd3e68 R08:  R09: 
> 0001
> [  101.840096] R10: 0008 R11: f000 R12: 
> 0001
> [  101.847260] R13: ffea R14: 0002 R15: 
> b9d00380
> [  101.854434] FS:  7f7e1f34a700() GS:8e733f8c() 
> knlGS:
> [  101.862518] CS:  0010 DS:  ES:  CR0: 80050033
> [  101.

[PATCH 1/1 v2] netfilter: constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.

This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l3/4proto structures const
subsequently.

Done with the help of Coccinelle.

Some spacing adjusted to fit within 80 characters.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

v2: Added consideration of array parameters.  This adds transformation of
nf_ct_l4proto_pernet_register and nf_ct_l4proto_pernet_unregister.

This patch also adds transformation of ctnl_timeout_parse_policy that was
somehow overlooked previously.

 include/net/netfilter/nf_conntrack_l3proto.h |6 ++---
 include/net/netfilter/nf_conntrack_l4proto.h |   14 ++--
 include/net/netfilter/nf_conntrack_timeout.h |2 -
 net/netfilter/nf_conntrack_core.c|8 +++
 net/netfilter/nf_conntrack_netlink.c |6 ++---
 net/netfilter/nf_conntrack_proto.c   |   30 +--
 net/netfilter/nfnetlink_cttimeout.c  |5 ++--
 7 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l3proto.h 
b/include/net/netfilter/nf_conntrack_l3proto.h
index 6d14b36..4782b15 100644
--- a/include/net/netfilter/nf_conntrack_l3proto.h
+++ b/include/net/netfilter/nf_conntrack_l3proto.h
@@ -76,17 +76,17 @@ struct nf_conntrack_l3proto {
 #ifdef CONFIG_SYSCTL
 /* Protocol pernet registration. */
 int nf_ct_l3proto_pernet_register(struct net *net,
- struct nf_conntrack_l3proto *proto);
+ const struct nf_conntrack_l3proto *proto);
 #else
 static inline int nf_ct_l3proto_pernet_register(struct net *n,
-   struct nf_conntrack_l3proto *p)
+   const struct nf_conntrack_l3proto *p)
 {
return 0;
 }
 #endif
 
 void nf_ct_l3proto_pernet_unregister(struct net *net,
-struct nf_conntrack_l3proto *proto);
+const struct nf_conntrack_l3proto *proto);
 
 /* Protocol global registration. */
 int nf_ct_l3proto_register(struct nf_conntrack_l3proto *proto);
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h 
b/include/net/netfilter/nf_conntrack_l4proto.h
index 7032e04..c86e946 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -125,23 +125,23 @@ struct nf_conntrack_l4proto 
*__nf_ct_l4proto_find(u_int16_t l3proto,
 
 struct nf_conntrack_l4proto *nf_ct_l4proto_find_get(u_int16_t l3proto,
u_int8_t l4proto);
-void nf_ct_l4proto_put(struct nf_conntrack_l4proto *p);
+void nf_ct_l4proto_put(const struct nf_conntrack_l4proto *p);
 
 /* Protocol pernet registration. */
 int nf_ct_l4proto_pernet_register_one(struct net *net,
- struct nf_conntrack_l4proto *proto);
+   const struct nf_conntrack_l4proto *proto);
 void nf_ct_l4proto_pernet_unregister_one(struct net *net,
-struct nf_conntrack_l4proto *proto);
+   const struct nf_conntrack_l4proto *proto);
 int nf_ct_l4proto_pernet_register(struct net *net,
- struct nf_conntrack_l4proto *proto[],
+ struct nf_conntrack_l4proto * const proto[],
  unsigned int num_proto);
 void nf_ct_l4proto_pernet_unregister(struct net *net,
-struct nf_conntrack_l4proto *proto[],
-unsigned int num_proto);
+   struct nf_conntrack_l4proto * const proto[],
+   unsigned int num_proto);
 
 /* Protocol global registration. */
 int nf_ct_l4proto_register_one(struct nf_conntrack_l4proto *proto);
-void nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *proto);
+void nf_ct_l4proto_unregister_one(const struct nf_conntrack_l4proto *proto);
 int nf_ct_l4proto_register(struct nf_conntrack_l4proto *proto[],
   unsigned int num_proto);
 void nf_ct_l4proto_unregister(struct nf_conntrack_l4proto *proto[],
diff --git a/include/net/netfilter/nf_conntrack_timeout.h 
b/include/net/netfilter/nf_conntrack_timeout.h
index d40b893..b222957 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -68,7 +68,7 @@ struct nf_conn_timeout *nf_ct_timeout_ext_add(struct nf_conn 
*ct,
 
 static inline unsigned int *
 nf_ct_timeout_lookup(struct net *net, struct nf_conn *ct,
-struct nf_conntrack_l4proto *l4proto)
+const struct nf_conntrack_l4proto *l

[PATCH 0/1 v2] constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.

This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l4proto structures const
subsequently.

Done with the help of Coccinelle.  The following semantic patch shows
the nf_conntrack_l4proto case.

// 
virtual update_results
virtual after_start

@initialize:ocaml@
@@

let unsafe = Hashtbl.create 101

let is_unsafe f = Hashtbl.mem unsafe f

let changed = ref false


(* The next three rules relate to the fact that we do not know the type of
void * variables.  Fortunately this is only neede on the first iteration,
but it still means that the whole kernel will end up being considered. *)

@has depends on !after_start && !update_results@
identifier f,x;
position p;
@@

f@p(...,struct nf_conntrack_l3proto *x,...) { ... }

@hasa depends on !after_start@
identifier f,x;
position p;
@@

f@p(...,struct nf_conntrack_l3proto *x[],...) { ... }

@others depends on !after_start && !update_results@
position p != {has.p,hasa.p};
identifier f,x;
@@

f@p(...,void *x,...) { ... }

@script:ocaml@
f << others.f;
@@

changed := true;
Hashtbl.add unsafe f ()


@fpb depends on !update_results disable optional_qualifier, drop_cast exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
type T;
@@

f(...,struct nf_conntrack_l3proto *x,...)
{
...
(
  return x;
|
  (<+...x...+>) aop e
|
  e aop x
|
  (T)x
|
  &(<+...x...+>)
|
  bad(...,x,...)
|
  fp(...,x,...)
|
  (<+...e->fld...+>)(...,x,...)
)
...when any
 }

@script:ocaml@
f << fpb.f;
@@

changed := true;
Printf.eprintf "%s is unsafe\n" f;
Hashtbl.add unsafe f ()

@fpba depends on !update_results disable optional_qualifier, drop_cast exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
type T;
@@

f(...,struct nf_conntrack_l3proto *x[],...)
{
...
(
  return \(x\|x[...]\);
|
  (<+...x...+>) aop e
|
  e aop \(x\|x[...]\)
|
  (T)\(x\|x[...]\)
|
  &(<+...x...+>)
|
  bad(...,\(x\|x[...]\),...)
|
  fp(...,\(x\|x[...]\),...)
|
  (<+...e->fld...+>)(...,\(x\|x[...]\),...)
)
... when any
 }

@script:ocaml@
f << fpba.f;
@@

changed := true;
Printf.eprintf "%s is unsafe\n" f;
Hashtbl.add unsafe f ()

@finalize:ocaml depends on !update_results@
tbls << merge.unsafe;
c << merge.changed;
@@

List.iter
(fun t ->
  Hashtbl.iter
(fun k v ->
  if not (Hashtbl.mem unsafe k) then Hashtbl.add unsafe k ()) t)
tbls;
changed := false;
let changed = List.exists (fun x -> !x) c in
let it = new iteration() in
it#add_virtual_rule After_start;
(if not changed
then it#add_virtual_rule Update_results);
it#register()

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
@@

f(...,
+ const
  struct nf_conntrack_l3proto *x,...) { ... }

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
type T;
@@

T f(...,
+ const
  struct nf_conntrack_l3proto *x,...);

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
@@

f(...,
+ const
  struct nf_conntrack_l3proto *x[],...) { ... }

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier x;
type T;
@@

T f(...,
+ const
  struct nf_conntrack_l3proto *x[],...);
// 

---

v2: Added consideration of array parameters.  This adds transformation of
nf_ct_l4proto_pernet_register and nf_ct_l4proto_pernet_unregister.

This patch also adds transformation of ctnl_timeout_parse_policy that was
somehow overlooked previously.

 include/net/netfilter/nf_conntrack_l3proto.h |6 ++---
 include/net/netfilter/nf_conntrack_l4proto.h |   14 ++--
 include/net/netfilter/nf_conntrack_timeout.h |2 -
 net/netfilter/nf_conntrack_core.c|8 +++
 net/netfilter/nf_conntrack_netlink.c |6 ++---
 net/netfilter/nf_conntrack_proto.c   |   30 +--
 net/netfilter/nfnetlink_cttimeout.c  |5 ++--
 7 files changed, 36 insertions(+), 35 deletions(-)

Re: [PATCH 1/1] netfilter: constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

On Sat, 29 Jul 2017, Florian Westphal wrote:

> Julia Lawall <julia.law...@lip6.fr> wrote:
> > When a nf_conntrack_l3/4proto parameter is not on the left hand side
> > of an assignment, its address is not taken, and it is not passed to a
> > function that may modify its fields, then it can be declared as const.
> >
> > This change is useful from a documentation point of view, and can
> > possibly facilitate making some nf_conntrack_l3/4proto structures const
> > subsequently.
> >
> > Done with the help of Coccinelle.
> >
> > Some spacing adjusted to fit within 80 characters.
>
> Acked-by: Florian Westphal <f...@strlen.de>
>
> Thanks Julia.
>
> I think we can indeed constify these completely after making
> 'nla_size' set at compile time.
>
> I'll send a simple attempt to make it so for l3proto soon.

There is another issue with respect to nf_ct_l3proto_unregister.  This
calls nf_ct_iterate_destroy with l3proto as the second argument.  This
function has signature:

void
nf_ct_iterate_destroy(int (*iter)(struct nf_conn *i, void *data), void *data)

The void * is not const.  Maybe it could be.

julia

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

[PATCH 0/1] constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.

This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l4proto structures const
subsequently.

Done with the help of Coccinelle.  The following semantic patch shows
the nf_conntrack_l4proto case.

// 
virtual update_results
virtual after_start

@initialize:ocaml@
@@

let unsafe = Hashtbl.create 101

let is_unsafe f = Hashtbl.mem unsafe f

let changed = ref false


(* The next three rules relate to the fact that we do not know the type of
void * variables.  Fortunately this is only needed on the first iteration,
but it still means that the whole kernel will end up being considered. *)

@has depends on !after_start@
identifier f,l4proto;
position p;
@@

f(...,struct nf_conntrack_l4proto *l4proto,...) { ... }

@others depends on !after_start@
position p != has.p;
identifier f,x;
@@

f(...,void *x,...) { ... }

@script:ocaml@
f << others.f;
@@

changed := true;
Hashtbl.add unsafe f ()


@fpb depends on !update_results disable optional_qualifier exists@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier l4proto,fld;
identifier bad : script:ocaml() { is_unsafe(bad) };
assignment operator aop;
expression e;
local idexpression fp;
@@

f(...,struct nf_conntrack_l4proto *l4proto,...)
{
<+...
(
  (<+...l4proto...+>) aop e
|
  &(<+...l4proto...+>)
|
  bad(...,l4proto,...)
|
  fp(...,l4proto,...)
|
  (<+...e->fld...+>)(...,l4proto,...)
)
...+> }

@script:ocaml@
f << fpb.f;
@@

changed := true;
Hashtbl.add unsafe f ()

@finalize:ocaml depends on !update_results@
tbls << merge.unsafe;
c << merge.changed;
@@

List.iter
(fun t ->
  Hashtbl.iter
(fun k v ->
  if not (Hashtbl.mem unsafe k) then Hashtbl.add unsafe k ()) t)
tbls;
let changed = List.exists (fun x -> !x) c in
let it = new iteration() in
it#add_virtual_rule After_start;
(if not changed
then it#add_virtual_rule Update_results);
it#register()

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier l4proto;
@@

f(...,
+ const
  struct nf_conntrack_l4proto *l4proto,...) { ... }

@depends on update_results disable optional_qualifier@
identifier f : script:ocaml() { not(is_unsafe(f)) };
identifier l4proto;
type T;
@@

T f(...,
+ const
  struct nf_conntrack_l4proto *l4proto,...);

// 

---

 include/net/netfilter/nf_conntrack_l3proto.h |6 +++---
 include/net/netfilter/nf_conntrack_l4proto.h |8 
 include/net/netfilter/nf_conntrack_timeout.h |2 +-
 net/netfilter/nf_conntrack_core.c|8 
 net/netfilter/nf_conntrack_netlink.c |6 +++---
 net/netfilter/nf_conntrack_proto.c   |   20 ++--
 net/netfilter/nfnetlink_cttimeout.c  |2 +-
 7 files changed, 26 insertions(+), 26 deletions(-)

[PATCH 1/1] netfilter: constify nf_conntrack_l3/4proto parameters

[Julia Lawall]

When a nf_conntrack_l3/4proto parameter is not on the left hand side
of an assignment, its address is not taken, and it is not passed to a
function that may modify its fields, then it can be declared as const.

This change is useful from a documentation point of view, and can
possibly facilitate making some nf_conntrack_l3/4proto structures const
subsequently.

Done with the help of Coccinelle.

Some spacing adjusted to fit within 80 characters.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 include/net/netfilter/nf_conntrack_l3proto.h |6 +++---
 include/net/netfilter/nf_conntrack_l4proto.h |8 
 include/net/netfilter/nf_conntrack_timeout.h |2 +-
 net/netfilter/nf_conntrack_core.c|8 
 net/netfilter/nf_conntrack_netlink.c |6 +++---
 net/netfilter/nf_conntrack_proto.c   |   20 ++--
 net/netfilter/nfnetlink_cttimeout.c  |2 +-
 7 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_l3proto.h 
b/include/net/netfilter/nf_conntrack_l3proto.h
index 6d14b36..4782b15 100644
--- a/include/net/netfilter/nf_conntrack_l3proto.h
+++ b/include/net/netfilter/nf_conntrack_l3proto.h
@@ -76,17 +76,17 @@ struct nf_conntrack_l3proto {
 #ifdef CONFIG_SYSCTL
 /* Protocol pernet registration. */
 int nf_ct_l3proto_pernet_register(struct net *net,
- struct nf_conntrack_l3proto *proto);
+ const struct nf_conntrack_l3proto *proto);
 #else
 static inline int nf_ct_l3proto_pernet_register(struct net *n,
-   struct nf_conntrack_l3proto *p)
+   const struct nf_conntrack_l3proto *p)
 {
return 0;
 }
 #endif
 
 void nf_ct_l3proto_pernet_unregister(struct net *net,
-struct nf_conntrack_l3proto *proto);
+const struct nf_conntrack_l3proto *proto);
 
 /* Protocol global registration. */
 int nf_ct_l3proto_register(struct nf_conntrack_l3proto *proto);
diff --git a/include/net/netfilter/nf_conntrack_l4proto.h 
b/include/net/netfilter/nf_conntrack_l4proto.h
index 7032e04..72589b7 100644
--- a/include/net/netfilter/nf_conntrack_l4proto.h
+++ b/include/net/netfilter/nf_conntrack_l4proto.h
@@ -125,13 +125,13 @@ struct nf_conntrack_l4proto 
*__nf_ct_l4proto_find(u_int16_t l3proto,
 
 struct nf_conntrack_l4proto *nf_ct_l4proto_find_get(u_int16_t l3proto,
u_int8_t l4proto);
-void nf_ct_l4proto_put(struct nf_conntrack_l4proto *p);
+void nf_ct_l4proto_put(const struct nf_conntrack_l4proto *p);
 
 /* Protocol pernet registration. */
 int nf_ct_l4proto_pernet_register_one(struct net *net,
- struct nf_conntrack_l4proto *proto);
+ const struct nf_conntrack_l4proto *proto);
 void nf_ct_l4proto_pernet_unregister_one(struct net *net,
-struct nf_conntrack_l4proto *proto);
+   const struct nf_conntrack_l4proto *proto);
 int nf_ct_l4proto_pernet_register(struct net *net,
  struct nf_conntrack_l4proto *proto[],
  unsigned int num_proto);
@@ -141,7 +141,7 @@ void nf_ct_l4proto_pernet_unregister(struct net *net,
 
 /* Protocol global registration. */
 int nf_ct_l4proto_register_one(struct nf_conntrack_l4proto *proto);
-void nf_ct_l4proto_unregister_one(struct nf_conntrack_l4proto *proto);
+void nf_ct_l4proto_unregister_one(const struct nf_conntrack_l4proto *proto);
 int nf_ct_l4proto_register(struct nf_conntrack_l4proto *proto[],
   unsigned int num_proto);
 void nf_ct_l4proto_unregister(struct nf_conntrack_l4proto *proto[],
diff --git a/include/net/netfilter/nf_conntrack_timeout.h 
b/include/net/netfilter/nf_conntrack_timeout.h
index d40b893..b222957 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -68,7 +68,7 @@ struct nf_conn_timeout *nf_ct_timeout_ext_add(struct nf_conn 
*ct,
 
 static inline unsigned int *
 nf_ct_timeout_lookup(struct net *net, struct nf_conn *ct,
-struct nf_conntrack_l4proto *l4proto)
+const struct nf_conntrack_l4proto *l4proto)
 {
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
struct nf_conn_timeout *timeout_ext;
diff --git a/net/netfilter/nf_conntrack_core.c 
b/net/netfilter/nf_conntrack_core.c
index 51390fe..ed4e04e 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1183,8 +1183,8 @@ void nf_conntrack_free(struct nf_conn *ct)
 static noinline struct nf_conntrack_tuple_hash *
 init_conntrack(struct net *net, struct nf_conn *tmpl,
   const struct nf_conntrack_tuple *tuple,
-  struct nf_conntrack_l3proto *l3proto,
-  

[PATCH] net-next: stmmac: dwmac-sun8i: fix of_table.cocci warnings

[Julia Lawall]

Make sure (of/i2c/platform)_device_id tables are NULL terminated

Generated by: scripts/coccinelle/misc/of_table.cocci

Fixes: d5dbe1976d52 ("net-next: stmmac: dwmac-sun8i: choose internal PHY via 
compatible")
CC: Corentin Labbe 
Signed-off-by: Fengguang Wu 
---

url:
https://github.com/0day-ci/linux/commits/Corentin-Labbe/dt-bindings-net-add-compatible-for-internal-sun8i-h3-sun8i-v3s-PHYs/20170729-174950
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

 dwmac-sun8i.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -892,6 +892,7 @@ static int sun8i_dwmac_probe(struct plat
static const struct of_device_id internal_phys[] = {
{ .compatible = "allwinner,sun8i-h3-ephy" },
{ .compatible = "allwinner,sun8i-v3s-ephy" },
+   {},
};

ret = stmmac_get_platform_resources(pdev, _res);

Re: [PATCH 0/2] constify nf_hook_ops structures

[Julia Lawall]

On Sat, 29 Jul 2017, Florian Westphal wrote:

> Julia Lawall <julia.law...@lip6.fr> wrote:
> > On Sat, 29 Jul 2017, Florian Westphal wrote:
> > > From a quick glance I don't see why we can't e.g. constify
> > > nf_conntrack_l3/4_proto too. It is not going to be as simple
> > > as just placing const everywhere, but I see no requirement for
> > > having these writeable.
> >
> > I will take a look.
>
> Thanks.

For the protos, the functions nf_ct_l3proto_register and
nf_ct_l4proto_register_one update the nla_size field. I don't know how
many structures reach these functions.

julia

>
> nf_logger and nf_loginfo also look like constify candidates.
>
> If there is a way to add "const" qualifier to pointer-to-structs
> that are not modified this would good as well to have IMO, if just
> for purpose of documentation.  For instance:
>
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -1177,8 +1177,8 @@ void nf_conntrack_free(struct nf_conn *ct)
>  static noinline struct nf_conntrack_tuple_hash *
>  init_conntrack(struct net *net, struct nf_conn *tmpl,
>const struct nf_conntrack_tuple *tuple,
> -  struct nf_conntrack_l3proto *l3proto,
> -  struct nf_conntrack_l4proto *l4proto,
> +  const struct nf_conntrack_l3proto *l3proto,
> +  const struct nf_conntrack_l4proto *l4proto,
>
>
> (its only passed as arg to a function that expects
> "const struct nf_conntrack_x *").
>
> I think we have several (also non-static helpers) that
> take "struct foo *" arg while they could use "const struct foo*"
> instead.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 0/2] constify nf_hook_ops structures

[Julia Lawall]

On Sat, 29 Jul 2017, Florian Westphal wrote:

> Julia Lawall <julia.law...@lip6.fr> wrote:
> >
> >
> > On Sat, 29 Jul 2017, Florian Westphal wrote:
> >
> > > Julia Lawall <julia.law...@lip6.fr> wrote:
> > > > The nf_hook_ops structure is only passed as the second argument to
> > > > nf_register_net_hook or nf_unregister_net_hook, both of which are
> > > > declared as const.  Thus the nf_hook_ops structure itself can be
> > > > const.
> > >
> > > Right, also see
> > > http://patchwork.ozlabs.org/patch/793767/
> > >
> > > This series misses most of them (all arrays perhaps)?
> >
> > Yes, my rule doesn't look for arrays.  I guess they are all done already
> > anyway?
>
> I think so (the patch is not yet applied though).

OK, just drop my patch then.

>
> From a quick glance I don't see why we can't e.g. constify
> nf_conntrack_l3/4_proto too. It is not going to be as simple
> as just placing const everywhere, but I see no requirement for
> having these writeable.

I will take a look.

thanks,
julia

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

Re: [PATCH 0/2] constify nf_hook_ops structures

[Julia Lawall]

On Sat, 29 Jul 2017, Florian Westphal wrote:

> Julia Lawall <julia.law...@lip6.fr> wrote:
> > The nf_hook_ops structure is only passed as the second argument to
> > nf_register_net_hook or nf_unregister_net_hook, both of which are
> > declared as const.  Thus the nf_hook_ops structure itself can be
> > const.
>
> Right, also see
> http://patchwork.ozlabs.org/patch/793767/
>
> This series misses most of them (all arrays perhaps)?

Yes, my rule doesn't look for arrays.  I guess they are all done already
anyway?

thanks,
julia


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

[PATCH 2/2] netfilter: ipt_CLUSTERIP: constify nf_hook_ops structures

[Julia Lawall]

The nf_hook_ops structure is only passed as the second argument to
nf_register_net_hook or nf_unregister_net_hook, both of which are
declared as const.  Thus the nf_hook_ops structure itself can be
const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct nf_hook_ops i@p = { ... };

@ok1@
identifier r.i;
expression e;
position p;
@@
 \(nf_register_net_hook\|nf_unregister_net_hook\)(e,@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct nf_hook_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct nf_hook_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/ipv4/netfilter/ipt_CLUSTERIP.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c 
b/net/ipv4/netfilter/ipt_CLUSTERIP.c
index efaa04d..17b4ca5 100644
--- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
+++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
@@ -625,7 +625,7 @@ static void arp_print(struct arp_payload *payload)
return NF_ACCEPT;
 }
 
-static struct nf_hook_ops cip_arp_ops __read_mostly = {
+static const struct nf_hook_ops cip_arp_ops = {
.hook = arp_mangle,
.pf = NFPROTO_ARP,
.hooknum = NF_ARP_OUT,

[PATCH 1/2] decnet: dn_rtmsg: constify nf_hook_ops structures

[Julia Lawall]

The nf_hook_ops structure is only passed as the second argument to
nf_register_net_hook or nf_unregister_net_hook, both of which are
declared as const.  Thus the nf_hook_ops structure itself can be
const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct nf_hook_ops i@p = { ... };

@ok1@
identifier r.i;
expression e;
position p;
@@
 \(nf_register_net_hook\|nf_unregister_net_hook\)(e,@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct nf_hook_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct nf_hook_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/decnet/netfilter/dn_rtmsg.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/decnet/netfilter/dn_rtmsg.c b/net/decnet/netfilter/dn_rtmsg.c
index aa8ffec..ab395e5 100644
--- a/net/decnet/netfilter/dn_rtmsg.c
+++ b/net/decnet/netfilter/dn_rtmsg.c
@@ -115,7 +115,7 @@ static inline void dnrmg_receive_user_skb(struct sk_buff 
*skb)
RCV_SKB_FAIL(-EINVAL);
 }
 
-static struct nf_hook_ops dnrmg_ops __read_mostly = {
+static const struct nf_hook_ops dnrmg_ops = {
.hook   = dnrmg_hook,
.pf = NFPROTO_DECNET,
.hooknum= NF_DN_ROUTE,

[PATCH 0/2] constify nf_hook_ops structures

[Julia Lawall]

The nf_hook_ops structure is only passed as the second argument to
nf_register_net_hook or nf_unregister_net_hook, both of which are
declared as const.  Thus the nf_hook_ops structure itself can be
const.

Done with the help of Coccinelle.

---

 net/decnet/netfilter/dn_rtmsg.c|2 +-
 net/ipv4/netfilter/ipt_CLUSTERIP.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

[PATCH 1/2] ipv4: constify net_protocol structures

[Julia Lawall]

The net_protocol structures are only passed as the first argument to
inet_add_protocol, which is declared as const.  Thus the net_protocol
structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct net_protocol i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
 inet_add_protocol(@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct net_protocol e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct net_protocol i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/ipv4/af_inet.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 5ce44fb..2e38624 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1594,7 +1594,7 @@ u64 snmp_fold_field64(void __percpu *mib, int offt, 
size_t syncp_offset)
 };
 #endif
 
-static struct net_protocol tcp_protocol = {
+static const struct net_protocol tcp_protocol = {
.early_demux=   tcp_v4_early_demux,
.early_demux_handler =  tcp_v4_early_demux,
.handler=   tcp_v4_rcv,
@@ -1604,7 +1604,7 @@ u64 snmp_fold_field64(void __percpu *mib, int offt, 
size_t syncp_offset)
.icmp_strict_tag_validation = 1,
 };
 
-static struct net_protocol udp_protocol = {
+static const struct net_protocol udp_protocol = {
.early_demux =  udp_v4_early_demux,
.early_demux_handler =  udp_v4_early_demux,
.handler =  udp_rcv,

[PATCH 0/2] constify net_protocol structures

[Julia Lawall]

The net_protocol structure is only passed as the first argument to
inet_add_protocol or inet_del_protocol, both of which are declared
as const.  Thus the net_protocol structure itself can be const.

Done with the help of Coccinelle.

---

 net/ipv4/af_inet.c |4 ++--
 net/l2tp/l2tp_ip.c |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

[PATCH 2/2] l2tp: constify net_protocol structures

[Julia Lawall]

The net_protocol structure is only passed as the first argument to
inet_add_protocol or inet_del_protocol, both of which are declared
as const.  Thus the net_protocol structure itself can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct net_protocol i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
 \(inet_add_protocol\|inet_del_protocol\)(@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct net_protocol e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct net_protocol i = { ... };
// 

Also drop __read_mostly.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/l2tp/l2tp_ip.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 4d322c1..c8e4d85 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -649,7 +649,7 @@ int l2tp_ioctl(struct sock *sk, int cmd, unsigned long arg)
.ops= _ip_ops,
 };
 
-static struct net_protocol l2tp_ip_protocol __read_mostly = {
+static const struct net_protocol l2tp_ip_protocol = {
.handler= l2tp_ip_recv,
.netns_ok   = 1,
 };

[PATCH 0/2] constify inet6_protocol structures

[Julia Lawall]

The inet6_protocol structure is only passed as the first argument to
inet6_add_protocol or inet6_del_protocol, both of which are declared as
const.  Thus the inet6_protocol structure itself can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@

static struct inet6_protocol i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@

 \(inet6_add_protocol\|inet6_del_protocol\)(@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct inet6_protocol e;
@@

e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@

static
+const
 struct inet6_protocol i = { ... };
// 

---

 net/ipv6/ip6_gre.c  |2 +-
 net/ipv6/tcp_ipv6.c |2 +-
 net/ipv6/udp.c  |2 +-
 net/l2tp/l2tp_ip6.c |2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

[PATCH 1/2] ipv6: constify inet6_protocol structures

[Julia Lawall]

The inet6_protocol structure is only passed as the first argument to
inet6_add_protocol or inet6_del_protocol, both of which are declared as
const.  Thus the inet6_protocol structure itself can be const.

Also drop __read_mostly where present on the newly const structures.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/ipv6/ip6_gre.c  |2 +-
 net/ipv6/tcp_ipv6.c |2 +-
 net/ipv6/udp.c  |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 67ff2aa..33865d6 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -1080,7 +1080,7 @@ static void ip6gre_fb_tunnel_init(struct net_device *dev)
 }
 
 
-static struct inet6_protocol ip6gre_protocol __read_mostly = {
+static const struct inet6_protocol ip6gre_protocol = {
.handler = gre_rcv,
.err_handler = ip6gre_err,
.flags   = INET6_PROTO_NOPOLICY|INET6_PROTO_FINAL,
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 90a3257..2968a33 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1945,7 +1945,7 @@ struct proto tcpv6_prot = {
.diag_destroy   = tcp_abort,
 };
 
-static struct inet6_protocol tcpv6_protocol = {
+static const struct inet6_protocol tcpv6_protocol = {
.early_demux=   tcp_v6_early_demux,
.early_demux_handler =  tcp_v6_early_demux,
.handler=   tcp_v6_rcv,
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 4a3e656..5f8b8d7 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1448,7 +1448,7 @@ int compat_udpv6_getsockopt(struct sock *sk, int level, 
int optname,
 }
 #endif
 
-static struct inet6_protocol udpv6_protocol = {
+static const struct inet6_protocol udpv6_protocol = {
.early_demux=   udp_v6_early_demux,
.early_demux_handler =  udp_v6_early_demux,
.handler=   udpv6_rcv,

[PATCH 2/2] l2tp: constify inet6_protocol structures

[Julia Lawall]

The inet6_protocol structure is only passed as the first argument to
inet6_add_protocol or inet6_del_protocol, both of which are declared as
const.  Thus the inet6_protocol structure itself can be const.

Also drop __read_mostly on the newly const structure.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 net/l2tp/l2tp_ip6.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 88b397c..d2efcd9 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -788,7 +788,7 @@ static int l2tp_ip6_recvmsg(struct sock *sk, struct msghdr 
*msg, size_t len,
.ops= _ip6_ops,
 };
 
-static struct inet6_protocol l2tp_ip6_protocol __read_mostly = {
+static const struct inet6_protocol l2tp_ip6_protocol = {
.handler= l2tp_ip6_recv,
 };
 

Re: [RFC] tls: length check correct in do_tls_getsockopt_tx()?

[Julia Lawall]

On Wed, 5 Jul 2017, Dan Carpenter wrote:

> On Wed, Jul 05, 2017 at 03:10:53AM -0400, Matthias Rosenfelder wrote:
> > Hi,
> >
> > In do_tls_getsockopt_tx():
> >
> > if (len == sizeof(crypto_info)) {
> >
> > should be
> >
> > if (len == sizeof(*crypto_info)) {
> >
> > as crypto_info is of pointer type. Or am I missing something?
>
> No, you're right.  It should be "sizeof(*crypto_info)".
>
> It's hard for Smatch to catch these sorts of bugs because Sparse
> sometimes just gives Smatch a number literal instead of a sizeof()
> expression.  I think Coccinelle can catch these kinds of bugs?

Currently, I think the Coccinelle rules are only looking for things like x
= f(...,sizeof(x),...)

julia

Re: [PATCH net-next] cxgb4: Add PTP Hardware Clock (PHC) support (fwd)

[Julia Lawall]

The complete context isn't shown, but it seems likely that there is a goto
out_free under line 1207, with no unlock on >ptp_lock.

julia

-- Forwarded message --
Date: Fri, 30 Jun 2017 11:54:23 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH net-next] cxgb4: Add PTP Hardware Clock (PHC) support

Hi Atul,

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Atul-Gupta/cxgb4-Add-PTP-Hardware-Clock-PHC-support/20170629-200758
:: branch date: 16 hours ago
:: commit date: 16 hours ago

>> drivers/net/ethernet/chelsio/cxgb4/sge.c:1190:2-8: preceding lock on line 
>> 1204

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 159226c60ceb77858018f6c31d17a575b3679b8a
vim +1190 drivers/net/ethernet/chelsio/cxgb4/sge.c

fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1184/*
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1185 * The chip min packet length is 10 octets but play 
safe and reject
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1186 * anything shorter than an Ethernet header.
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1187 */
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1188if (unlikely(skb->len < ETH_HLEN)) {
a7525198a drivers/net/ethernet/chelsio/cxgb4/sge.c Eric W. Biederman
2014-03-15  1189  out_free: dev_kfree_skb_any(skb);
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01 @1190return NETDEV_TX_OK;
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1191}
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1192
637d3e997 drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2015-05-05  1193/* Discard the packet if the length is greater than mtu 
*/
637d3e997 drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2015-05-05  1194max_pkt_len = ETH_HLEN + dev->mtu;
8d09e6b8b drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2016-07-28  1195if (skb_vlan_tagged(skb))
637d3e997 drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2015-05-05  1196max_pkt_len += VLAN_HLEN;
637d3e997 drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2015-05-05  1197if (!skb_shinfo(skb)->gso_size && (unlikely(skb->len > 
max_pkt_len)))
637d3e997 drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2015-05-05  1198goto out_free;
637d3e997 drivers/net/ethernet/chelsio/cxgb4/sge.c Hariprasad Shenai
2015-05-05  1199
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1200pi = netdev_priv(dev);
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1201adap = pi->adapter;
fd3a47900 drivers/net/cxgb4/sge.c  Dimitris Michailidis 
2010-04-01  1202qidx = skb_get_queue_mapping(skb);
159226c60 drivers/net/ethernet/chelsio/cxgb4/sge.c Atul Gupta   
2017-06-28  1203if (ptp_enabled) {
159226c60 drivers/net/ethernet/chelsio/cxgb4/sge.c Atul Gupta   
2017-06-28 @1204spin_lock(>ptp_lock);
159226c60 drivers/net/ethernet/chelsio/cxgb4/sge.c Atul Gupta   
2017-06-28  1205if (!(adap->ptp_tx_skb)) {
159226c60 drivers/net/ethernet/chelsio/cxgb4/sge.c Atul Gupta   
2017-06-28  1206skb_shinfo(skb)->tx_flags |= 
SKBTX_IN_PROGRESS;
159226c60 drivers/net/ethernet/chelsio/cxgb4/sge.c Atul Gupta   
2017-06-28  1207adap->ptp_tx_skb = skb_get(skb);

:: The code at line 1190 was first introduced by commit
:: fd3a47900b6f9fa72a4074ecb630f9dae62f1a95 cxgb4: Add packet queues and 
packet DMA code

:: TO: Dimitris Michailidis <d...@chelsio.com>
:: CC: David S. Miller <da...@davemloft.net>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH net 3/3] netvsc: fix RCU warning from set_multicast (fwd)

[Julia Lawall]

Please check the indentation of lines 73 onward.

thanks,
julia

-- Forwarded message --
Date: Tue, 6 Jun 2017 14:40:59 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH net 3/3] netvsc: fix RCU warning from set_multicast

CC: kbuild-...@01.org
In-Reply-To: <20170605211010.9571-4-sthem...@microsoft.com>
TO: Stephen Hemminger <step...@networkplumber.org>
CC: da...@davemloft.net
CC: netdev@vger.kernel.org, Stephen Hemminger <sthem...@microsoft.com>

Hi Stephen,

[auto build test WARNING on net/master]

url:
https://github.com/0day-ci/linux/commits/Stephen-Hemminger/netvsc-bug-fixes/20170606-120730
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/net/hyperv/netvsc_drv.c:71:2-26: code aligned with following code on 
>> line 73

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 65bcf34d2ae4e8a34c6b65dbd9efff1aee7050a2
vim +71 drivers/net/hyperv/netvsc_drv.c

65bcf34d Stephen Hemminger 2017-06-05  65   struct netvsc_device *nvdev;
d426b2e3 Haiyang Zhang 2011-11-30  66   struct rndis_device *rdev;
d426b2e3 Haiyang Zhang 2011-11-30  67
65bcf34d Stephen Hemminger 2017-06-05  68   rtnl_lock();
65bcf34d Stephen Hemminger 2017-06-05  69   nvdev = 
rtnl_dereference(ndevctx->nvdev);
65bcf34d Stephen Hemminger 2017-06-05  70   if (nvdev)
d426b2e3 Haiyang Zhang 2011-11-30 @71   rdev = nvdev->extension;
d426b2e3 Haiyang Zhang 2011-11-30  72
65bcf34d Stephen Hemminger 2017-06-05 @73   if (rdev) {
0a1275ca Vitaly Kuznetsov  2016-05-13  74   if (ndev->flags 
& IFF_PROMISC)
d426b2e3 Haiyang Zhang 2011-11-30  75   
rndis_filter_set_packet_filter(rdev,
d426b2e3 Haiyang Zhang 2011-11-30  76   
   NDIS_PACKET_TYPE_PROMISCUOUS);

:: The code at line 71 was first introduced by commit
:: d426b2e3d91f8ec3203f8852e7ad0153b5dfdf71 net/hyperv: Add support for 
promiscuous mode setting

:: TO: Haiyang Zhang <haiya...@microsoft.com>
:: CC: Greg Kroah-Hartman <gre...@suse.de>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH][V2] net: stmmac: ensure jumbo_frm error return is correctly checked for -ve value

[Julia Lawall]

On Mon, 5 Jun 2017, Colin King wrote:

> From: Colin Ian King 
>
> The current comparison of entry < 0 will never be true since entry is an
> unsigned integer. Make entry an int to ensure -ve error return values
> from the call to jumbo_frm are correctly being caught.

Perhaps it would be good to mention that the return value of jumbo_frm
is an int and that the subsequent computations on entry are all on small
values so there is no need for unsigned.

julia

>
> Detected by CoverityScan, CID#1238760 ("Macro compares unsigned to 0")
>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 68a188e74c54..3346a99ce4c4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2945,7 +2945,8 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, 
> struct net_device *dev)
>   int i, csum_insertion = 0, is_jumbo = 0;
>   u32 queue = skb_get_queue_mapping(skb);
>   int nfrags = skb_shinfo(skb)->nr_frags;
> - unsigned int entry, first_entry;
> + int entry;
> + unsigned int first_entry;
>   struct dma_desc *desc, *first;
>   struct stmmac_tx_queue *tx_q;
>   unsigned int enh_desc;
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] net: stmmac: ensure jumbo_frm error return is correctly checked for -ve value

[Julia Lawall]

On Sat, 3 Jun 2017, Colin Ian King wrote:

> On 03/06/17 16:55, Andy Shevchenko wrote:
> > On Fri, Jun 2, 2017 at 5:58 PM, Colin King  wrote:
> >> The current comparison of entry < 0 will never be true since entry is an
> >> unsigned integer. Cast entry to an int to ensure -ve error return values
> >> from the call to jumbo_frm are correctly being caught.
> >
> >> if (unlikely(is_jumbo) && likely(priv->synopsys_id <
> >>  DWMAC_CORE_4_00)) {
> >> entry = priv->hw->mode->jumbo_frm(tx_q, skb, 
> >> csum_insertion);
> >> -   if (unlikely(entry < 0))
> >> +   if (unlikely((int)entry < 0))
> >
> > It feels like a hiding some other issue.
> >
>
> The alternative is:
>
>   int rc = priv->hw->mode->jumbo_frm(tx_q, skb, csum_insertion);
>   if (unlikely(rc < 0))
>   goto dma_map_err;
>
>   entry = rc;
>
> however, that is effectively the same. The cast I'm using is a well used
> idiom in the kernel, it used in almost a hundred similar cases.
>
> git grep "< 0" | grep "(int)" | wc -l
> 95

Does entry really have to be unsigned?  The jumbo_frm function returns an
int, not an unsigned int, so it seems unpleasant to make it unsigned
prematurely just to put a cast afterwards.  The remaining computation
seems to involve only small numbers.

julia

[PATCH] dsa: mv88e6xxx: fix returnvar.cocci warnings

[Julia Lawall]

Remove unneeded variable used to store return value.

Generated by: scripts/coccinelle/misc/returnvar.cocci

CC: Andrew Lunn <and...@lunn.ch>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---

It's a minor issue, but since there is no error, the code is a bit
misleading.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
master
head:   47936d35edbac5e58064bd15e51136050b2f2717
commit: 04aca9938255fc7097b3fb5700f408524656f2e2 [330/362] dsa: mv88e6xxx:
Enable/Disable SERDES on port enable/disable

 chip.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -1982,13 +1982,12 @@ static int mv88e6xxx_port_enable(struct
 struct phy_device *phydev)
 {
struct mv88e6xxx_chip *chip = ds->priv;
-   int err = 0;

mutex_lock(>reg_lock);
mv88e6xxx_serdes_power(chip, port, true);
mutex_unlock(>reg_lock);

-   return err;
+   return 0;
 }

 static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port,

Re: [PATCH v3 1/2] net: phy: Add Cortina CS4340 driver (fwd)

[Julia Lawall]

The u32 values on lines 73 and 79 will not be less than 0.

julia

-- Forwarded message --
Date: Fri, 26 May 2017 19:31:28 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v3 1/2] net: phy: Add Cortina CS4340 driver

CC: kbuild-...@01.org
In-Reply-To: <1495785519-1468-2-git-send-email-bogdan.purcare...@nxp.com>
TO: Bogdan Purcareata <bogdan.purcare...@nxp.com>
CC: and...@lunn.ch, f.faine...@gmail.com, netdev@vger.kernel.org, 
devicet...@vger.kernel.org, linux-ker...@vger.kernel.org
CC:

Hi Bogdan,

[auto build test WARNING on net-next/master]
[also build test WARNING on v4.12-rc2 next-20170526]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Bogdan-Purcareata/net-phy-Add-Cortina-CS4340-driver/20170526-170949
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/net/phy/cortina.c:73:5-11: WARNING: Unsigned expression compared 
>> with zero: id_lsb < 0
>> drivers/net/phy/cortina.c:79:5-11: WARNING: Unsigned expression compared 
>> with zero: id_msb < 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 84281ef4be227acc6a2f638da7a6842f7f55348d
vim +73 drivers/net/phy/cortina.c

84281ef4 Bogdan Purcareata 2017-05-26  67  static int cortina_probe(struct 
phy_device *phydev)
84281ef4 Bogdan Purcareata 2017-05-26  68  {
84281ef4 Bogdan Purcareata 2017-05-26  69   u32 phy_id = 0, id_lsb, id_msb;
84281ef4 Bogdan Purcareata 2017-05-26  70
84281ef4 Bogdan Purcareata 2017-05-26  71   /* Read device id from phy 
registers. */
84281ef4 Bogdan Purcareata 2017-05-26  72   id_lsb = 
cortina_read_reg(phydev, VILLA_GLOBAL_CHIP_ID_LSB);
84281ef4 Bogdan Purcareata 2017-05-26 @73   if (id_lsb < 0)
84281ef4 Bogdan Purcareata 2017-05-26  74   return -ENXIO;
84281ef4 Bogdan Purcareata 2017-05-26  75
84281ef4 Bogdan Purcareata 2017-05-26  76   phy_id = id_lsb << 16;
84281ef4 Bogdan Purcareata 2017-05-26  77
84281ef4 Bogdan Purcareata 2017-05-26  78   id_msb = 
cortina_read_reg(phydev, VILLA_GLOBAL_CHIP_ID_MSB);
84281ef4 Bogdan Purcareata 2017-05-26 @79   if (id_msb < 0)
84281ef4 Bogdan Purcareata 2017-05-26  80   return -ENXIO;
84281ef4 Bogdan Purcareata 2017-05-26  81
84281ef4 Bogdan Purcareata 2017-05-26  82   phy_id |= id_msb;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

[net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with zero: unfrag_ip6hlen < 0 (fwd)

[Julia Lawall]

Hello,

It may be worth checking on these.  The code context is shown in the first
case (line 120).  For the others, at least it gives the line numbers.

julia

-- Forwarded message --
Date: Thu, 18 May 2017 09:07:31 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: [net:master 9/12] net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned
expression compared with zero: unfrag_ip6hlen < 0

CC: kbuild-...@01.org
CC: netdev@vger.kernel.org
TO: Craig Gallek <kr...@google.com>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git master
head:   579f1d926c66cc0bd3bd87b1fe2e091084b07430
commit: 2423496af35d94a87156b063ea5cedffc10a70a1 [9/12] ipv6: Prevent overrun 
when parsing v6 header options
:: branch date: 2 hours ago
:: commit date: 6 hours ago

>> net/ipv6/ip6_offload.c:120:7-21: WARNING: Unsigned expression compared with 
>> zero: unfrag_ip6hlen < 0
--
>> net/ipv6/ip6_output.c:601:5-9: WARNING: Unsigned expression compared with 
>> zero: hlen < 0
--
>> net/ipv6/udp_offload.c:94:6-20: WARNING: Unsigned expression compared with 
>> zero: unfrag_ip6hlen < 0

git remote add net https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git
git remote update net
git checkout 2423496af35d94a87156b063ea5cedffc10a70a1
vim +120 net/ipv6/ip6_offload.c

d1da932e Vlad Yasevich2012-11-15  104
07b26c94 Steffen Klassert 2016-09-19  105   gso_partial = 
!!(skb_shinfo(segs)->gso_type & SKB_GSO_PARTIAL);
07b26c94 Steffen Klassert 2016-09-19  106
d1da932e Vlad Yasevich2012-11-15  107   for (skb = segs; skb; skb = 
skb->next) {
d3e5e006 Eric Dumazet 2013-10-20  108   ipv6h = (struct ipv6hdr 
*)(skb_mac_header(skb) + nhoff);
07b26c94 Steffen Klassert 2016-09-19  109   if (gso_partial)
802ab55a Alexander Duyck  2016-04-10  110   payload_len = 
skb_shinfo(skb)->gso_size +
802ab55a Alexander Duyck  2016-04-10  111 
SKB_GSO_CB(skb)->data_offset +
802ab55a Alexander Duyck  2016-04-10  112 
skb->head - (unsigned char *)(ipv6h + 1);
802ab55a Alexander Duyck  2016-04-10  113   else
802ab55a Alexander Duyck  2016-04-10  114   payload_len = 
skb->len - nhoff - sizeof(*ipv6h);
802ab55a Alexander Duyck  2016-04-10  115   ipv6h->payload_len = 
htons(payload_len);
d3e5e006 Eric Dumazet 2013-10-20  116   skb->network_header = 
(u8 *)ipv6h - skb->head;
d3e5e006 Eric Dumazet 2013-10-20  117
91a48a2e Hannes Frederic Sowa 2014-02-24  118   if (udpfrag) {
d1da932e Vlad Yasevich2012-11-15  119   unfrag_ip6hlen 
= ip6_find_1stfragopt(skb, );
2423496a Craig Gallek 2017-05-16 @120   if 
(unfrag_ip6hlen < 0)
2423496a Craig Gallek 2017-05-16  121   return 
ERR_PTR(unfrag_ip6hlen);
d3e5e006 Eric Dumazet 2013-10-20  122   fptr = (struct 
frag_hdr *)((u8 *)ipv6h + unfrag_ip6hlen);
d1da932e Vlad Yasevich2012-11-15  123   fptr->frag_off 
= htons(offset);
53b24b8f Ian Morris   2015-03-29  124   if (skb->next)
d1da932e Vlad Yasevich2012-11-15  125   
fptr->frag_off |= htons(IP6_MF);
d1da932e Vlad Yasevich2012-11-15  126   offset += 
(ntohs(ipv6h->payload_len) -
d1da932e Vlad Yasevich2012-11-15  127  
sizeof(struct frag_hdr));
d1da932e Vlad Yasevich2012-11-15  128   }

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH] mdio: mux: fix device_node_continue.cocci warnings

[Julia Lawall]

On Fri, 12 May 2017, Florian Fainelli wrote:

> On 05/12/2017 09:22 AM, David Miller wrote:
> > From: Julia Lawall <julia.law...@lip6.fr>
> > Date: Fri, 12 May 2017 22:54:23 +0800 (SGT)
> >
> >> Device node iterators put the previous value of the index variable, so an
> >> explicit put causes a double put.
> >  ...
> >> @@ -169,7 +169,6 @@ int mdio_mux_init(struct device *dev,
> >>if (r) {
> >>mdiobus_free(cb->mii_bus);
> >>devm_kfree(dev, cb);
> >> -  of_node_put(child_bus_node);
> >>} else {
> >
> > I think we're instead simply missing a break; statement here.
>
> It's kind of questionable, if we have an error initializing one of our
> child MDIO bus controller (child from the perspective of the MDIO mux,
> boy this is getting complicated...), should we keep on going, or should
> we abort entirely and rollback what we have successfully registered?
>
> I don't think Julia's patch makes thing worse, in that if we had to
> rollback, we would not be doing this correctly now anyway.

Just to be clear, if you want the break instead, then you need to keep the
put.

julia

>
> Jon, what do you think?
> --
> Florian
>

[PATCH] mdio: mux: fix device_node_continue.cocci warnings

[Julia Lawall]

Device node iterators put the previous value of the index variable, so an
explicit put causes a double put.

In particular, of_mdiobus_register can fail before doing anything
interesting, so one could view it as a no-op from the reference count
point of view.

Generated by: scripts/coccinelle/iterators/device_node_continue.cocci

CC: Jon Mason <jon.ma...@broadcom.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---

tree:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
master
head:   8785ded64cfb68b8d8b2583c7c1fc611f99eabf2
commit: b60161668199ac62011c024adc9e66713b9554e7 [13999/14120] mdio: mux:

 mdio-mux.c |1 -
 1 file changed, 1 deletion(-)

--- a/drivers/net/phy/mdio-mux.c
+++ b/drivers/net/phy/mdio-mux.c
@@ -169,7 +169,6 @@ int mdio_mux_init(struct device *dev,
if (r) {
mdiobus_free(cb->mii_bus);
devm_kfree(dev, cb);
-   of_node_put(child_bus_node);
} else {
cb->next = pb->children;
pb->children = cb;

Re: [PATCH] net: dsa: loop: Free resources if initialization is deferred

[Julia Lawall]

On Wed, 10 May 2017, Christophe JAILLET wrote:

> Free some devm'allocated memory in case of deferred driver initialization.
> This avoid to waste some memory in such a case.

I really think it would be helpful to mention the special behavior of
-EPROBE_DEFER.  It doesn't take much space, and it coud be helpful to
someone in the future.

julia

>
> Suggested-by: Joe Perches 
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/net/dsa/dsa_loop.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
> index a19e1781e9bb..557afb418320 100644
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -260,8 +260,11 @@ static int dsa_loop_drv_probe(struct mdio_device 
> *mdiodev)
>   return -ENOMEM;
>
>   ps->netdev = dev_get_by_name(_net, pdata->netdev);
> - if (!ps->netdev)
> + if (!ps->netdev) {
> + devm_kfree(>dev, ps);
> + devm_kfree(>dev, ds);
>   return -EPROBE_DEFER;
> + }
>
>   pdata->cd.netdev[DSA_LOOP_CPU_PORT] = >netdev->dev;
>
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] net: dsa: loop: Check for memory allocation failure

[Julia Lawall]

On Mon, 8 May 2017, Florian Fainelli wrote:

> On 05/08/2017 04:46 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 8 May 2017, Joe Perches wrote:
> >
> >> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
> >>>
> >>> On Mon, 8 May 2017, David Laight wrote:
> >>>
> >>>> From: Christophe JAILLET
> >>>>> Sent: 06 May 2017 06:30
> >>>>> If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> >>>>> Return -ENOMEM instead, as done for some other memory allocation just a
> >>>>> few lines above.
> >>>>
> >>>> ...
> >>>>> --- a/drivers/net/dsa/dsa_loop.c
> >>>>> +++ b/drivers/net/dsa/dsa_loop.c
> >>>>> @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device 
> >>>>> *mdiodev)
> >>>>> return -ENOMEM;
> >>>>>
> >>>>> ps = devm_kzalloc(>dev, sizeof(*ps), GFP_KERNEL);
> >>>>> +   if (!ps)
> >>>>> +   return -ENOMEM;
> >>>>> +
> >>>>> ps->netdev = dev_get_by_name(_net, pdata->netdev);
> >>>>> if (!ps->netdev)
> >>>>> return -EPROBE_DEFER;
> >>>>
> >>>> On the face if it this code leaks like a sieve.
> >>>
> >>> I don't think so.  The allocations (dsa_switch_alloc and devm_kzalloc) use
> >>> devm functions.
> >>
> >> It's at least wasteful.
> >>
> >> Each time -EPROBE_DEFER occurs, another set of calls to
> >> dsa_switch_alloc and dev_kzalloc also occurs.
> >>
> >> Perhaps it'd be better to do:
> >>
> >>if (ps->netdev) {
> >>devm_kfree(>dev, ps);
> >>devm_kfree(>dev, ds);
> >>return -EPROBE_DEFER;
> >>}
> >
> > Is EPROBE_DEFER handled differently than other kinds of errors?
>
> In the core device driver model, yes, EPROBE_DEFER is treated
> differently than other errors because it puts the driver on a retry queue.
>
> EPROBE_DEFER is already a slow and exceptional path, and this is a
> mock-up driver, so I am not sure what value there is in trying to
> balance devm_kzalloc() with corresponding devm_kfree()...

OK, thanks for the explanation.

julia

Re: [PATCH] net: dsa: loop: Check for memory allocation failure

[Julia Lawall]

On Mon, 8 May 2017, Joe Perches wrote:

> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
> >
> > On Mon, 8 May 2017, David Laight wrote:
> >
> > > From: Christophe JAILLET
> > > > Sent: 06 May 2017 06:30
> > > > If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> > > > Return -ENOMEM instead, as done for some other memory allocation just a
> > > > few lines above.
> > >
> > > ...
> > > > --- a/drivers/net/dsa/dsa_loop.c
> > > > +++ b/drivers/net/dsa/dsa_loop.c
> > > > @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device 
> > > > *mdiodev)
> > > > return -ENOMEM;
> > > >
> > > > ps = devm_kzalloc(>dev, sizeof(*ps), GFP_KERNEL);
> > > > +   if (!ps)
> > > > +   return -ENOMEM;
> > > > +
> > > > ps->netdev = dev_get_by_name(_net, pdata->netdev);
> > > > if (!ps->netdev)
> > > > return -EPROBE_DEFER;
> > >
> > > On the face if it this code leaks like a sieve.
> >
> > I don't think so.  The allocations (dsa_switch_alloc and devm_kzalloc) use
> > devm functions.
>
> It's at least wasteful.
>
> Each time -EPROBE_DEFER occurs, another set of calls to
> dsa_switch_alloc and dev_kzalloc also occurs.
>
> Perhaps it'd be better to do:
>
>   if (ps->netdev) {
>   devm_kfree(>dev, ps);
>   devm_kfree(>dev, ds);
>   return -EPROBE_DEFER;
>   }

Is EPROBE_DEFER handled differently than other kinds of errors?

julia


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

RE: [PATCH] net: dsa: loop: Check for memory allocation failure

[Julia Lawall]

On Mon, 8 May 2017, David Laight wrote:

> From: Christophe JAILLET
> > Sent: 06 May 2017 06:30
> > If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> > Return -ENOMEM instead, as done for some other memory allocation just a
> > few lines above.
> ...
> > --- a/drivers/net/dsa/dsa_loop.c
> > +++ b/drivers/net/dsa/dsa_loop.c
> > @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device 
> > *mdiodev)
> > return -ENOMEM;
> >
> > ps = devm_kzalloc(>dev, sizeof(*ps), GFP_KERNEL);
> > +   if (!ps)
> > +   return -ENOMEM;
> > +
> > ps->netdev = dev_get_by_name(_net, pdata->netdev);
> > if (!ps->netdev)
> > return -EPROBE_DEFER;
>
> On the face if it this code leaks like a sieve.

I don't think so.  The allocations (dsa_switch_alloc and devm_kzalloc) use
devm functions.

julia

Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)

[Julia Lawall]

It looks like >cmdr_lock should be released at line 512 if it has
not been released otherwise.  The lock was taken at line 438.

julia

-- Forwarded message --
Date: Fri, 14 Apr 2017 22:21:44 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
call sites

Hi Logan,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc6]
[cannot apply to next-20170413]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 8 hours ago
:: commit date: 8 hours ago

>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
   drivers/target/target_core_user.c:512:2-8: preceding lock on line 471

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
vim +512 drivers/target/target_core_user.c

7c9e7a6f Andy Grover  2014-10-01  432   
sizeof(struct tcmu_cmd_entry));
7c9e7a6f Andy Grover  2014-10-01  433   command_size = base_command_size
7c9e7a6f Andy Grover  2014-10-01  434   + 
round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
7c9e7a6f Andy Grover  2014-10-01  435
7c9e7a6f Andy Grover  2014-10-01  436   WARN_ON(command_size & 
(TCMU_OP_ALIGN_SIZE-1));
7c9e7a6f Andy Grover  2014-10-01  437
7c9e7a6f Andy Grover  2014-10-01 @438   spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  439
7c9e7a6f Andy Grover  2014-10-01  440   mb = udev->mb_addr;
7c9e7a6f Andy Grover  2014-10-01  441   cmd_head = mb->cmd_head % 
udev->cmdr_size; /* UAM */
26418649 Sheng Yang   2016-02-26  442   data_length = 
se_cmd->data_length;
26418649 Sheng Yang   2016-02-26  443   if (se_cmd->se_cmd_flags & 
SCF_BIDI) {
26418649 Sheng Yang   2016-02-26  444   
BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
26418649 Sheng Yang   2016-02-26  445   data_length += 
se_cmd->t_bidi_data_sg->length;
26418649 Sheng Yang   2016-02-26  446   }
554617b2 Andy Grover  2016-08-25  447   if ((command_size > 
(udev->cmdr_size / 2)) ||
554617b2 Andy Grover  2016-08-25  448   data_length > 
udev->data_size) {
554617b2 Andy Grover  2016-08-25  449   pr_warn("TCMU: Request 
of size %zu/%zu is too big for %u/%zu "
3d9b9555 Andy Grover  2016-08-25  450   "cmd ring/data 
area\n", command_size, data_length,
7c9e7a6f Andy Grover  2014-10-01  451   
udev->cmdr_size, udev->data_size);
554617b2 Andy Grover  2016-08-25  452   
spin_unlock_irq(>cmdr_lock);
554617b2 Andy Grover  2016-08-25  453   return 
TCM_INVALID_CDB_FIELD;
554617b2 Andy Grover  2016-08-25  454   }
7c9e7a6f Andy Grover  2014-10-01  455
26418649 Sheng Yang   2016-02-26  456   while 
(!is_ring_space_avail(udev, command_size, data_length)) {
7c9e7a6f Andy Grover  2014-10-01  457   int ret;
7c9e7a6f Andy Grover  2014-10-01  458   DEFINE_WAIT(__wait);
7c9e7a6f Andy Grover  2014-10-01  459
7c9e7a6f Andy Grover  2014-10-01  460   
prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
7c9e7a6f Andy Grover  2014-10-01  461
7c9e7a6f Andy Grover  2014-10-01  462   pr_debug("sleeping for 
ring space\n");
7c9e7a6f Andy Grover  2014-10-01  463   
spin_unlock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  464   ret = 
schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
7c9e7a6f Andy Grover  2014-10-01  465   
finish_wait(>wait_cmdr, &__wait);
7c9e7a6f Andy Grover  2014-10-01  466   if (!ret) {
7c9e7a6f Andy Grover  2014-10-01  467   pr_warn("tcmu: 
command timed out\n");
02eb924f Andy Grover  2016-10-06  468   return 
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
7c9e7a6f Andy Grover  2014-10-01  469   }
7c9e7a6f Andy Grover  2014-10-01  470
7c9e7a6f Andy Grover  2014-10-01  471   
spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  472
7c9e7a6f Andy Grover  2014-10-01  473   /* We dropped 
cmdr_lock, cmd_head is stale */
7c9e7a6f Andy Grover  2014-10-01  474   cmd_head = mb->cmd_head 
% udev->cmdr_size; /* UAM */
7c9e7a6f Andy Grover

Re: [PATCH 2/4 v2 net-next] net: stmmac: adding multiple buffers for RX (fwd)

[Julia Lawall]

It looks like an infinite loop, if queue is unsigned.

julia

-- Forwarded message --
Date: Thu, 6 Apr 2017 11:49:01 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 2/4 v2 net-next] net: stmmac: adding multiple buffers for RX

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Joao-Pinto/net-stmmac-break-some-functions-into-RX-and-TX-scopes/20170406-081523
:: branch date: 4 hours ago
:: commit date: 4 hours ago

   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:2852:15-20: WARNING: 
Unsigned expression compared with zero: entry < 0
>> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:1190:8-13: WARNING: 
>> Unsigned expression compared with zero: queue >= 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout b403b5806dca0f85904ddc6cf2241286da86ed9b
vim +1190 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c

b6ad2502 Joao Pinto 2017-04-05  1174if 
(priv->extend_desc)
b403b580 Joao Pinto 2017-04-05  1175
priv->hw->mode->init(rx_q->dma_erx,
b403b580 Joao Pinto 2017-04-05  1176
 rx_q->dma_rx_phy,
e3ad57c9 Giuseppe Cavallaro 2016-02-29  1177
 DMA_RX_SIZE, 1);
b6ad2502 Joao Pinto 2017-04-05  1178else
b403b580 Joao Pinto 2017-04-05  1179
priv->hw->mode->init(rx_q->dma_rx,
b403b580 Joao Pinto 2017-04-05  1180
 rx_q->dma_rx_phy,
e3ad57c9 Giuseppe Cavallaro 2016-02-29  1181
 DMA_RX_SIZE, 0);
b6ad2502 Joao Pinto 2017-04-05  1182}
b403b580 Joao Pinto 2017-04-05  1183}
b403b580 Joao Pinto 2017-04-05  1184
b403b580 Joao Pinto 2017-04-05  1185buf_sz = bfsize;
b6ad2502 Joao Pinto 2017-04-05  1186
b6ad2502 Joao Pinto 2017-04-05  1187return 0;
b403b580 Joao Pinto 2017-04-05  1188
b6ad2502 Joao Pinto 2017-04-05  1189  err_init_rx_buffers:
b403b580 Joao Pinto 2017-04-05 @1190while (queue >= 0) {
b6ad2502 Joao Pinto 2017-04-05  1191while (--i >= 0)
b403b580 Joao Pinto 2017-04-05  1192
stmmac_free_rx_buffer(priv, queue, i);
b403b580 Joao Pinto 2017-04-05  1193
b403b580 Joao Pinto 2017-04-05  1194i = DMA_RX_SIZE;
b403b580 Joao Pinto 2017-04-05  1195queue--;
b403b580 Joao Pinto 2017-04-05  1196}
b403b580 Joao Pinto 2017-04-05  1197
b6ad2502 Joao Pinto 2017-04-05  1198return ret;

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH net-next v6 01/11] bpf: Add eBPF program subtype and is_valid_subtype() verifier (fwd)

[Julia Lawall]

Size is unsigned, so not negative.

julia

-- Forwarded message --
Date: Wed, 29 Mar 2017 23:06:01 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH net-next v6 01/11] bpf: Add eBPF program subtype and
is_valid_subtype() verifier

In-Reply-To: <20170328234650.19695-2-...@digikod.net>
TO: "Mickaël Salaün" <m...@digikod.net>

Hi Mickaël,

[auto build test WARNING on net-next/master]

url:
https://github.com/0day-ci/linux/commits/Micka-l-Sala-n/Landlock-LSM-Toward-unprivileged-sandboxing/20170329-211258
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> kernel/bpf/syscall.c:1041:5-9: WARNING: Unsigned expression compared with 
>> zero: size < 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 07d282aef4f60235407284c0be81d01e352e040b
vim +1041 kernel/bpf/syscall.c

f4324551 Daniel Mack2016-11-23  1025return -EINVAL;
f4324551 Daniel Mack2016-11-23  1026}
f4324551 Daniel Mack2016-11-23  1027
7f677633 Alexei Starovoitov 2017-02-10  1028return ret;
f4324551 Daniel Mack2016-11-23  1029  }
f4324551 Daniel Mack2016-11-23  1030  #endif /* CONFIG_CGROUP_BPF */
f4324551 Daniel Mack2016-11-23  1031
99c55f7d Alexei Starovoitov 2014-09-26  1032  SYSCALL_DEFINE3(bpf, int, cmd, 
union bpf_attr __user *, uattr, unsigned int, size)
99c55f7d Alexei Starovoitov 2014-09-26  1033  {
99c55f7d Alexei Starovoitov 2014-09-26  1034union bpf_attr attr = {};
99c55f7d Alexei Starovoitov 2014-09-26  1035int err;
99c55f7d Alexei Starovoitov 2014-09-26  1036
1be7f75d Alexei Starovoitov 2015-10-07  1037if (!capable(CAP_SYS_ADMIN) && 
sysctl_unprivileged_bpf_disabled)
99c55f7d Alexei Starovoitov 2014-09-26  1038return -EPERM;
99c55f7d Alexei Starovoitov 2014-09-26  1039
07d282ae Mickaël Salaün 2017-03-29  1040size = check_user_buf((void 
__user *)uattr, size, sizeof(attr));
07d282ae Mickaël Salaün 2017-03-29 @1041if (size < 0)
07d282ae Mickaël Salaün 2017-03-29  1042return size;
99c55f7d Alexei Starovoitov 2014-09-26  1043
99c55f7d Alexei Starovoitov 2014-09-26  1044/* copy attributes from user 
space, may be less than sizeof(bpf_attr) */
99c55f7d Alexei Starovoitov 2014-09-26  1045if (copy_from_user(, 
uattr, size) != 0)
99c55f7d Alexei Starovoitov 2014-09-26  1046return -EFAULT;
99c55f7d Alexei Starovoitov 2014-09-26  1047
99c55f7d Alexei Starovoitov 2014-09-26  1048switch (cmd) {
99c55f7d Alexei Starovoitov 2014-09-26  1049case BPF_MAP_CREATE:

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [Outreachy kernel] [PATCH] net: netfilter: remove unused variable

[Julia Lawall]

On Wed, 29 Mar 2017, Arushi Singhal wrote:

> This patch uses the following coccinelle script to remove
> a variable that was simply used to store the return
> value of a function call before returning it:
>
> @@
> identifier len,f;
> @@
>
> -int len;
>  ... when != len
>  when strict
> -len =
> +return
> f(...);
> -return len;
>
> Signed-off-by: Arushi Singhal 
> ---
>  net/netfilter/ipvs/ip_vs_ftp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c
> index d30c327bb578..9e1e682610ef 100644
> --- a/net/netfilter/ipvs/ip_vs_ftp.c
> +++ b/net/netfilter/ipvs/ip_vs_ftp.c
> @@ -482,11 +482,9 @@ static struct pernet_operations ip_vs_ftp_ops = {
>
>  static int __init ip_vs_ftp_init(void)
>  {
> - int rv;
>
> - rv = register_pernet_subsys(_vs_ftp_ops);
>   /* rcu_barrier() is called by netns on error */
> - return rv;
> + return register_pernet_subsys(_vs_ftp_ops);

It looks like you end up with an unnecessary blank line at the beginning
of the function.

julia

>  }
>
>  /*
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170329133848.GA1336%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] net: Remove unnecessary cast on void pointer

[Julia Lawall]

On Tue, 28 Mar 2017, simran singhal wrote:

> The following Coccinelle script was used to detect this:
> @r@
> expression x;
> void* e;
> type T;
> identifier f;
> @@
> (
>   *((T *)e)
> |
>   ((T *)x)[...]
> |
>   ((T*)x)->f
> |
>
> - (T*)
>   e
> )
>
> Signed-off-by: simran singhal 
> ---
>  net/bridge/netfilter/ebtables.c |  2 +-
>  net/ipv4/netfilter/arp_tables.c | 20 
>  net/ipv4/netfilter/ip_tables.c  | 20 
>  net/ipv6/netfilter/ip6_tables.c | 20 
>  net/netfilter/ipset/ip_set_bitmap_gen.h |  4 ++--
>  net/netfilter/ipset/ip_set_core.c   |  2 +-
>  net/netfilter/nf_conntrack_proto.c  |  2 +-
>  net/netfilter/nft_set_hash.c|  2 +-
>  net/netfilter/xt_hashlimit.c| 10 +-
>  9 files changed, 35 insertions(+), 47 deletions(-)
>
> diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
> index 79b6991..bdc629e 100644
> --- a/net/bridge/netfilter/ebtables.c
> +++ b/net/bridge/netfilter/ebtables.c
> @@ -1713,7 +1713,7 @@ static int compat_copy_entry_to_user(struct ebt_entry 
> *e, void __user **dstptr,
>   if (*size < sizeof(*ce))
>   return -EINVAL;
>
> - ce = (struct ebt_entry __user *)*dstptr;
> + ce = *dstptr;
>   if (copy_to_user(ce, e, sizeof(*ce)))
>   return -EFAULT;
>
> diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
> index 6241a81..f046c12 100644
> --- a/net/ipv4/netfilter/arp_tables.c
> +++ b/net/ipv4/netfilter/arp_tables.c
> @@ -310,7 +310,7 @@ static int mark_source_chains(const struct xt_table_info 
> *newinfo,
>   for (hook = 0; hook < NF_ARP_NUMHOOKS; hook++) {
>   unsigned int pos = newinfo->hook_entry[hook];
>   struct arpt_entry *e
> - = (struct arpt_entry *)(entry0 + pos);
> + = (entry0 + pos);

The parentheses are not needed here.  The newline is not needed either.

>
>   if (!(valid_hooks & (1 << hook)))
>   continue;
> @@ -354,14 +354,12 @@ static int mark_source_chains(const struct 
> xt_table_info *newinfo,
>   if (pos == oldpos)
>   goto next;
>
> - e = (struct arpt_entry *)
> - (entry0 + pos);
> + e = (entry0 + pos);

Again, drop the parentheses.

>   } while (oldpos == pos + e->next_offset);
>
>   /* Move along one */
>   size = e->next_offset;
> - e = (struct arpt_entry *)
> - (entry0 + pos + size);
> + e = (entry0 + pos + size);

And again.

>   if (pos + size >= newinfo->size)
>   return 0;
>   e->counters.pcnt = pos;
> @@ -376,16 +374,14 @@ static int mark_source_chains(const struct 
> xt_table_info *newinfo,
>   if (!xt_find_jump_offset(offsets, 
> newpos,
>
> newinfo->number))
>   return 0;
> - e = (struct arpt_entry *)
> - (entry0 + newpos);
> + e = (entry0 + newpos);

etc :)

julia

>   } else {
>   /* ... this is a fallthru */
>   newpos = pos + e->next_offset;
>   if (newpos >= newinfo->size)
>   return 0;
>   }
> - e = (struct arpt_entry *)
> - (entry0 + newpos);
> + e = (entry0 + newpos);
>   e->counters.pcnt = pos;
>   pos = newpos;
>   }
> @@ -683,7 +679,7 @@ static int copy_entries_to_user(unsigned int total_size,
>   for (off = 0, num = 0; off < total_size; off += e->next_offset, num++){
>   const struct xt_entry_target *t;
>
> - e = (struct arpt_entry *)(loc_cpu_entry + off);
> + e = (loc_cpu_entry + off);
>   if (copy_to_user(userptr + off, e, sizeof(*e))) {
>   ret = -EFAULT;
>   goto free_counters;
> @@ -1130,7 +1126,7 @@ compat_copy_entry_from_user(struct compat_arpt_entry 
> *e, void **dstptr,
>   int h;
>
>   origsize = *size;
> - de = (struct arpt_entry *)*dstptr;
> + de = *dstptr;
>   memcpy(de, e, sizeof(struct 

Re: [PATCH net-next 1/8] net: stmicro: multiple queues dt configuration (fwd)

[Julia Lawall]

This is just pointing out that lines 200 and 202 are identical.

julia

-- Forwarded message --
Date: Fri, 10 Mar 2017 04:50:01 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH net-next 1/8] net: stmicro: multiple queues dt configuration

In-Reply-To: 
<eee93719e5830cfea8e4dcfdac9221a3b6124eb8.1488969672.git.jpi...@synopsys.com>

Hi Joao,

[auto build test WARNING on next-20170308]
[also build test WARNING on v4.11-rc1]
[cannot apply to v4.9-rc8 v4.9-rc7 v4.9-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Joao-Pinto/prepare-mac-operations-for-multiple-queues/20170310-013207
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:199:6-8: WARNING: 
>> possible condition with no effect (if == else)

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout faca7330d9494884feb52b62a037d01266b4d382
vim +199 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c

faca7330 Joao Pinto 2017-03-08  183 /* TODO: Dynamic mapping to be 
included in the future */
faca7330 Joao Pinto 2017-03-08  184
faca7330 Joao Pinto 2017-03-08  185 queue++;
faca7330 Joao Pinto 2017-03-08  186 }
faca7330 Joao Pinto 2017-03-08  187
faca7330 Joao Pinto 2017-03-08  188 /* Processing TX queues common config */
faca7330 Joao Pinto 2017-03-08  189 if (of_property_read_u8(tx_node, 
"snps,tx-queues-to-use",
faca7330 Joao Pinto 2017-03-08  190 
>tx_queues_to_use))
faca7330 Joao Pinto 2017-03-08  191 plat->tx_queues_to_use = 1;
faca7330 Joao Pinto 2017-03-08  192
faca7330 Joao Pinto 2017-03-08  193 if (of_property_read_bool(tx_node, 
"snps,tx-sched-wrr"))
faca7330 Joao Pinto 2017-03-08  194 plat->tx_sched_algorithm = 
MTL_TX_ALGORITHM_WRR;
faca7330 Joao Pinto 2017-03-08  195 else if (of_property_read_bool(tx_node, 
"snps,tx-sched-wfq"))
faca7330 Joao Pinto 2017-03-08  196 plat->tx_sched_algorithm = 
MTL_TX_ALGORITHM_WFQ;
faca7330 Joao Pinto 2017-03-08  197 else if (of_property_read_bool(tx_node, 
"snps,tx-sched-dwrr"))
faca7330 Joao Pinto 2017-03-08  198 plat->tx_sched_algorithm = 
MTL_TX_ALGORITHM_DWRR;
faca7330 Joao Pinto 2017-03-08 @199 else if (of_property_read_bool(tx_node, 
"snps,tx-sched-sp"))
faca7330 Joao Pinto 2017-03-08  200 plat->tx_sched_algorithm = 
MTL_TX_ALGORITHM_SP;
faca7330 Joao Pinto 2017-03-08  201 else
faca7330 Joao Pinto 2017-03-08  202 plat->tx_sched_algorithm = 
MTL_TX_ALGORITHM_SP;
faca7330 Joao Pinto 2017-03-08  203
faca7330 Joao Pinto 2017-03-08  204 queue = 0;
faca7330 Joao Pinto 2017-03-08  205
faca7330 Joao Pinto 2017-03-08  206 /* Processing individual TX queue 
config */
faca7330 Joao Pinto 2017-03-08  207 for_each_child_of_node(tx_node, 
queue_node) {

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [patch] net/mlx4: && vs & typo

[Julia Lawall]

On Tue, 28 Feb 2017, Bart Van Assche wrote:

> On 02/28/2017 02:23 PM, Joe Perches wrote:
> > On Tue, 2017-02-28 at 15:35 +, Bart Van Assche wrote:
> >> On Tue, 2017-02-28 at 15:02 +0300, Dan Carpenter wrote:
> >>> Bitwise & was obviously intended here.
> > []
> >>> diff --git a/include/linux/mlx4/driver.h b/include/linux/mlx4/driver.h
> > []
> >>> @@ -109,7 +109,7 @@ static inline void(u8 *addr, u64 mac)
> >>>   int i;
> >>>
> >>>   for (i = ETH_ALEN; i > 0; i--) {
> >>> - addr[i - 1] = mac && 0xFF;
> >>> + addr[i - 1] = mac & 0xFF;
> >>>   mac >>= 8;
> >>>   }
> >>>  }
> >>
> >> Is this the only place where such a loop occurs?
> >
> > Seems to be.
> >
> >> Should a put_unaligned_be48()
> >> function be introduced?
> >
> > Why?  This is used exactly once.
>
> Really? Here is an example of another open-coded version of
> put_unaligned_be48() from arch/mips/cavium-octeon/octeon-platform.c:
>
>   new_mac[0] = (mac >> 40) & 0xff;
>   new_mac[1] = (mac >> 32) & 0xff;
>   new_mac[2] = (mac >> 24) & 0xff;
>   new_mac[3] = (mac >> 16) & 0xff;
>   new_mac[4] = (mac >> 8) & 0xff;
>   new_mac[5] = mac & 0xff;

drivers/media/radio/radio-shark2.c:
for (i = 0; i < 6; i++)
   shark->transfer_buffer[i + 1] = (reg >> (40 - i * 8)) & 0xff;

drivers/rtc/rtc-ab3100.c
   buf[0] = (hw_counter) & 0xFF;
   buf[1] = (hw_counter >> 8) & 0xFF;
   buf[2] = (hw_counter >> 16) & 0xFF;
   buf[3] = (hw_counter >> 24) & 0xFF;
   buf[4] = (hw_counter >> 32) & 0xFF;
   buf[5] = (hw_counter >> 40) & 0xFF;

drivers/net/ethernet/sun/ldmvsw.c
for (i = 0; i < ETH_ALEN; i++)
   port->raddr[i] = (*rmac >> (5 - i) * 8) & 0xff;

drivers/net/ethernet/sun/sunvnet.c
for (i = 0; i < ETH_ALEN; i++)
   dev->dev_addr[i] = (*local_mac >> (5 - i) * 8) & 0xff;

for (i = 0; i < ETH_ALEN; i++)
   port->raddr[i] = (*rmac >> (5 - i) * 8) & 0xff;

julia

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

[PATCH] rtlwifi: btcoexist: fix semicolon.cocci warnings

[Julia Lawall]

Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

CC: Larry Finger <larry.fin...@lwfinger.net>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
---

 halbtc8821a2ant.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c
+++ b/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c
@@ -2877,7 +2877,7 @@ static void halbtc8821a2ant_action_pan_e
  false, false);
btc8821a2ant_sw_mech2(btcoexist, false, false,
  false, 0x18);
-   };
+   }
} else {
/* fw mechanism */
if ((bt_rssi_state == BTC_RSSI_STATE_HIGH) ||

Re: [PATCH v5 2/2] net: dsa: mv88e6xxx: Add support for ethernet switch 88E6341 (fwd)

[Julia Lawall]

mv88e6xxx_6341_family is checked twice, on line 2606 and 2607.

julia

-- Forwarded message --
Date: Fri, 20 Jan 2017 19:38:12 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v5 2/2] net: dsa: mv88e6xxx: Add support for ethernet switch
 88E6341

In-Reply-To: <20170119214934.27442-3-gregory.clem...@free-electrons.com>

Hi Gregory,

[auto build test WARNING on net-next/master]
[also build test WARNING on next-20170120]
[cannot apply to v4.10-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Gregory-CLEMENT/Add-support-for-the-ethernet-switch-on-the-ESPRESSObin/20170120-180348
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/net/dsa/mv88e6xxx/chip.c:2606:36-63: duplicated argument to && or ||

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c618c22093235e96adf0c6f39497eeed083a60cf
vim +2606 drivers/net/dsa/mv88e6xxx/chip.c

0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2590 
if (err)
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2591 
return err;
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2592
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2593 
/* Egress rate control 2: disable egress rate control. */
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2594 
err = mv88e6xxx_port_write(chip, port, PORT_RATE_CONTROL_2, 0x);
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2595 
if (err)
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2596 
return err;
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2597
b35d322a1 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-12-03  2598 
if (chip->info->ops->port_pause_config) {
b35d322a1 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-12-03  2599 
err = chip->info->ops->port_pause_config(chip, port);
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2600 
if (err)
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2601 
return err;
b35d322a1 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-12-03  2602 
}
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2603
b35d322a1 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-12-03  2604 
if (mv88e6xxx_6352_family(chip) || mv88e6xxx_6351_family(chip) ||
b35d322a1 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-12-03  2605 
mv88e6xxx_6165_family(chip) || mv88e6xxx_6097_family(chip) ||
c618c2209 drivers/net/dsa/mv88e6xxx/chip.c Gregory CLEMENT 2017-01-19 @2606 
mv88e6xxx_6320_family(chip) || mv88e6xxx_6341_family(chip) ||
c618c2209 drivers/net/dsa/mv88e6xxx/chip.c Gregory CLEMENT 2017-01-19  2607 
mv88e6xxx_6341_family(chip)) {
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2608 
/* Port ATU control: disable limiting the number of
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2609 
 * address database entries that this port is allowed
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2610 
 * to use.
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2611 
 */
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2612 
err = mv88e6xxx_port_write(chip, port, PORT_ATU_CONTROL,
0e7b99257 drivers/net/dsa/mv88e6xxx/chip.c Andrew Lunn 2016-09-21  2613 
   0x);
54d792f25 drivers/net/dsa/mv88e6xxx.c  Andrew Lunn 2015-05-06  2614 
/* Priority Override: disable DA, SA and VTU priority

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [patch net-next] stmmac: indent an if statement

[Julia Lawall]

On Tue, 17 Jan 2017, Alexandre Torgue wrote:

> Hi Julia
>
> On 01/16/2017 11:10 PM, Julia Lawall wrote:
> >
> >
> > On Tue, 17 Jan 2017, Dan Carpenter wrote:
> >
> > > On Mon, Jan 16, 2017 at 10:46:22PM +0100, Julia Lawall wrote:
> > > >
> > > >
> > > > On Mon, 16 Jan 2017, Dan Carpenter wrote:
> > > >
> > > > > On Mon, Jan 16, 2017 at 12:19:24PM +0300, Dan Carpenter wrote:
> > > > > > On Sun, Jan 15, 2017 at 10:14:38PM -0500, David Miller wrote:
> > > > > > > From: Dan Carpenter <dan.carpen...@oracle.com>
> > > > > > > Date: Thu, 12 Jan 2017 21:46:32 +0300
> > > > > > >
> > > > > > > > The break statement should be indented one more tab.
> > > > > > > >
> > > > > > > > Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
> > > > > > >
> > > > > > > Applied, but like Julia I think we might have a missing
> > > > > > > of_node_put()
> > > > > > > here.
> > > > > >
> > > > > > Of course, sorry for dropping the ball on this.  I'll send a patch
> > > > > > for
> > > > > > that.
> > > > > >
> > > > >
> > > > > Actually, I've looked at it some more and I think this function is OK.
> > > > > We're supposed to do an of_node_put() later...  I can't find where
> > > > > that
> > > > > happens, but presumably that's because I don't know stmmac well.  This
> > > > > code here, though, is fine.
> > > >
> > > > Why do you think it is fine?  Does anyone in the calling context know
> > > > which child would have caused the break?
> > >
> > > Yeah.  It's saved in plat->mdio_node and we expect to be holding on
> > > either path through the function.
> > >
> > > (It would be better if one of the stmmac people were responding here
> > > insead of a random fix the indenting weenie like myself.)
> >
> > OK, I agree that there should not be an of_node_put with the break.
> >
> > Perhaps there should be an of_node_put on plat->mdio_node in
> > stmmac_remove_config_dt, like there is an of_node_put on plat->phy_node.
> > But it would certainly be helpful to hear from someone who knows the code
> > better.
>
> I also think it's missing! Can you propose a patch ?

Done.  Thanks for the clarification.

julia

[PATCH] stmmac: add missing of_node_put

[Julia Lawall]

The function stmmac_dt_phy provides several possibilities for initializing
plat->mdio_node, all of which have the effect of increasing the reference
count of the assigned value.  This field is not updated elsewhere, so the
value is live until the end of the lifetime of plat (devm_allocated), just
after the end of stmmac_remove_config_dt.  Thus, add an of_node_put on
plat->mdio_node in stmmac_remove_config_dt.  It is possible that the field
mdio_node is never initialized, but of_node_put is NULL-safe, so it is also
safe to call of_node_put in that case.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 4daa8a3..460f94f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -409,6 +409,7 @@ void stmmac_remove_config_dt(struct platform_device *pdev,
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
of_node_put(plat->phy_node);
+   of_node_put(plat->mdio_node);
 }
 #else
 struct plat_stmmacenet_data *