* [PATCH 1/2] net, vxlan Fix compile warning [v4]
@ 2013-10-01 13:24 Prarit Bhargava
2013-10-01 13:24 ` [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4] Prarit Bhargava
0 siblings, 1 reply; 5+ messages in thread
From: Prarit Bhargava @ 2013-10-01 13:24 UTC (permalink / raw)
To: netdev; +Cc: Prarit Bhargava, jpirko
Fix a unintialized variable warning.
drivers/net/vxlan.c: In function ‘vxlan_sock_add’:
drivers/net/vxlan.c:2240:11: error: ‘sock’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
vs->sock = sock;
^
drivers/net/vxlan.c:2217:17: note: ‘sock’ was declared here
struct socket *sock;
^
[v2]: davem suggested resolving this by making create_v{4,6}_sock() return an err
pointer.
[v3]: Ben Hutchings pointed out a missed conversion
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: jpirko@redhat.com
---
drivers/net/vxlan.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d1292fe..cdc78b4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2182,7 +2182,7 @@ static void vxlan_del_work(struct work_struct *work)
* could be used for both IPv4 and IPv6 communications, but
* users may set bindv6only=1.
*/
-static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v6_sock(struct net *net, __be16 port)
{
struct sock *sk;
struct socket *sock;
@@ -2195,7 +2195,7 @@ static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
rc = sock_create_kern(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, &sock);
if (rc < 0) {
pr_debug("UDPv6 socket create failed\n");
- return rc;
+ return ERR_PTR(rc);
}
/* Put in proper namespace */
@@ -2210,28 +2210,27 @@ static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
pr_debug("bind for UDPv6 socket %pI6:%u (%d)\n",
&vxlan_addr.sin6_addr, ntohs(vxlan_addr.sin6_port), rc);
sk_release_kernel(sk);
- return rc;
+ return ERR_PTR(rc);
}
/* At this point, IPv6 module should have been loaded in
* sock_create_kern().
*/
BUG_ON(!ipv6_stub);
- *psock = sock;
/* Disable multicast loopback */
inet_sk(sk)->mc_loop = 0;
- return 0;
+ return sock;
}
#else
-static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v6_sock(struct net *net, __be16 port)
{
- return -EPFNOSUPPORT;
+ return ERR_PTR(-EPFNOSUPPORT);
}
#endif
-static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v4_sock(struct net *net, __be16 port)
{
struct sock *sk;
struct socket *sock;
@@ -2246,7 +2245,7 @@ static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
rc = sock_create_kern(AF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
if (rc < 0) {
pr_debug("UDP socket create failed\n");
- return rc;
+ return ERR_PTR(rc);
}
/* Put in proper namespace */
@@ -2259,13 +2258,12 @@ static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
pr_debug("bind for UDP socket %pI4:%u (%d)\n",
&vxlan_addr.sin_addr, ntohs(vxlan_addr.sin_port), rc);
sk_release_kernel(sk);
- return rc;
+ return ERR_PTR(rc);
}
- *psock = sock;
/* Disable multicast loopback */
inet_sk(sk)->mc_loop = 0;
- return 0;
+ return sock;
}
/* Create new listen socket if needed */
@@ -2276,7 +2274,6 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
struct vxlan_sock *vs;
struct socket *sock;
struct sock *sk;
- int rc = 0;
unsigned int h;
vs = kmalloc(sizeof(*vs), GFP_KERNEL);
@@ -2289,12 +2286,12 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
INIT_WORK(&vs->del_work, vxlan_del_work);
if (ipv6)
- rc = create_v6_sock(net, port, &sock);
+ sock = create_v6_sock(net, port);
else
- rc = create_v4_sock(net, port, &sock);
- if (rc < 0) {
+ sock = create_v4_sock(net, port);
+ if (IS_ERR(sock)) {
kfree(vs);
- return ERR_PTR(rc);
+ return ERR_CAST(sock);
}
vs->sock = sock;
--
1.7.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4]
2013-10-01 13:24 [PATCH 1/2] net, vxlan Fix compile warning [v4] Prarit Bhargava
@ 2013-10-01 13:24 ` Prarit Bhargava
2013-10-01 15:17 ` Jack Morgenstein
0 siblings, 1 reply; 5+ messages in thread
From: Prarit Bhargava @ 2013-10-01 13:24 UTC (permalink / raw)
To: netdev; +Cc: Prarit Bhargava, dledford, amirv, davem, ogerlitz, jackm
Fix unitialized variable warnings.
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
atomic_dec(&cq->mtt->ref_count);
^
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
atomic_dec(&srq->mtt->ref_count);
[v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
pointer instead of setting 'cq' by reference. I also did the same for
srq.
Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: dledford@redhat.com
Cc: amirv@mellanox.com
Cc: davem@davemloft.net
Cc: ogerlitz@mellanox.com
Cc: jackm@dev.mellanox.co.il
---
.../net/ethernet/mellanox/mlx4/resource_tracker.c | 46 ++++++++++----------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index dd68763..343206b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
return err;
}
-static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
- enum res_cq_states state, struct res_cq **cq)
+static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
+ int slave, int cqn,
+ enum res_cq_states state)
{
struct mlx4_priv *priv = mlx4_priv(dev);
struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
r->com.from_state = r->com.state;
r->com.to_state = state;
r->com.state = RES_CQ_BUSY;
- if (cq)
- *cq = r;
+ } else {
+ r = ERR_PTR(err);
}
}
spin_unlock_irq(mlx4_tlock(dev));
- return err;
+ return r;
}
-static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
- enum res_cq_states state, struct res_srq **srq)
+static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
+ int index,
+ enum res_cq_states state)
{
struct mlx4_priv *priv = mlx4_priv(dev);
struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
r->com.from_state = r->com.state;
r->com.to_state = state;
r->com.state = RES_SRQ_BUSY;
- if (srq)
- *srq = r;
+ } else {
+ r = ERR_PTR(err);
}
}
spin_unlock_irq(mlx4_tlock(dev));
- return err;
+ return r;
}
static void res_abort_move(struct mlx4_dev *dev, int slave,
@@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
struct res_cq *cq;
struct res_mtt *mtt;
- err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
- if (err)
- return err;
+ cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
+ if (IS_ERR(cq))
+ return PTR_ERR(cq);
err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
if (err)
goto out_move;
@@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
int cqn = vhcr->in_modifier;
struct res_cq *cq;
- err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
- if (err)
- return err;
+ cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
+ if (IS_ERR(cq))
+ return PTR_ERR(cq);
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto out_move;
@@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
return -EINVAL;
- err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
- if (err)
- return err;
+ srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+ if (IS_ERR(srq))
+ return PTR_ERR(srq);
err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
if (err)
goto ex_abort;
@@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
int srqn = vhcr->in_modifier;
struct res_srq *srq;
- err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
- if (err)
- return err;
+ srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+ if (IS_ERR(srq))
+ return PTR_ERR(srq);
err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
if (err)
goto ex_abort;
--
1.7.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4]
2013-10-01 13:24 ` [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4] Prarit Bhargava
@ 2013-10-01 15:17 ` Jack Morgenstein
2013-10-01 15:25 ` Prarit Bhargava
0 siblings, 1 reply; 5+ messages in thread
From: Jack Morgenstein @ 2013-10-01 15:17 UTC (permalink / raw)
To: Prarit Bhargava; +Cc: netdev, dledford, amirv, davem, ogerlitz
On Tue, 1 Oct 2013 09:24:37 -0400
Prarit Bhargava <prarit@redhat.com> wrote:
Hi,
You missed some needed changes (You did not get compile warnings,
because indeed "r" was initialized in the paths below. However,
in these error paths, "err" was ignored.
You should be setting "r" to the error return values:
diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c index
343206b..c4a0a32 100644 ---
a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c +++
b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c @@ -1085,9
+1085,9 @@ static struct res_cq *cq_res_start_move_to(struct mlx4_dev
*dev,
spin_lock_irq(mlx4_tlock(dev));
r = res_tracker_lookup(&tracker->res_tree[RES_CQ], cqn);
if (!r)
- err = -ENOENT;
+ r = ERR_PTR(-ENOENT);
else if (r->com.owner != slave)
- err = -EPERM;
+ r = ERR_PTR(-EPERM);
else {
switch (state) {
case RES_CQ_BUSY:
@@ -1140,9 +1140,9 @@ static struct res_srq
*srq_res_start_move_to(struct mlx4_dev *dev, int slave,
spin_lock_irq(mlx4_tlock(dev));
r = res_tracker_lookup(&tracker->res_tree[RES_SRQ], index);
if (!r)
- err = -ENOENT;
+ r = ERR_PTR(-ENOENT);
else if (r->com.owner != slave)
- err = -EPERM;
+ r = ERR_PTR(-EPERM);
else {
switch (state) {
case RES_SRQ_BUSY:
There are also other calls which need to be changed in a similar
fashion (eq_res_start_move_to(), and one or two others -- I'm
surprised that these others did not also generate uninitialized-var
warnings). If we change cq and srq, we should also change the others.
Since we are in the middle of submitting patches which touch the file
"resource_tracker.c", I would really like to hold off on these warning
fixes for a bit, and I'll handle the changes for all the functions at
once to conform (correctly!) to the format suggested by Dave Miller.
-Jack
> Fix unitialized variable warnings.
>
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function
> ‘mlx4_HW2SW_CQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error:
> ‘cq’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized] atomic_dec(&cq->mtt->ref_count); ^
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function
> ‘mlx4_HW2SW_SRQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error:
> ‘srq’ may be used uninitialized in this function
> [-Werror=maybe-uninitialized] atomic_dec(&srq->mtt->ref_count);
>
> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an
> error pointer instead of setting 'cq' by reference. I also did the
> same for srq.
>
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: dledford@redhat.com
> Cc: amirv@mellanox.com
> Cc: davem@davemloft.net
> Cc: ogerlitz@mellanox.com
> Cc: jackm@dev.mellanox.co.il
> ---
> .../net/ethernet/mellanox/mlx4/resource_tracker.c | 46
> ++++++++++---------- 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c index
> dd68763..343206b 100644 ---
> a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c +++
> b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c @@ -1073,8
> +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int
> slave, int index, return err; }
>
> -static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int
> cqn,
> - enum res_cq_states state, struct
> res_cq **cq) +static struct res_cq *cq_res_start_move_to(struct
> mlx4_dev *dev,
> + int slave, int cqn,
> + enum res_cq_states
> state) {
> struct mlx4_priv *priv = mlx4_priv(dev);
> struct mlx4_resource_tracker *tracker =
> &priv->mfunc.master.res_tracker; @@ -1117,18 +1118,19 @@ static int
> cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
> r->com.from_state = r->com.state; r->com.to_state = state;
> r->com.state = RES_CQ_BUSY;
> - if (cq)
> - *cq = r;
> + } else {
> + r = ERR_PTR(err);
> }
> }
>
> spin_unlock_irq(mlx4_tlock(dev));
>
> - return err;
> + return r;
> }
>
> -static int srq_res_start_move_to(struct mlx4_dev *dev, int slave,
> int index,
> - enum res_cq_states state, struct
> res_srq **srq) +static struct res_srq *srq_res_start_move_to(struct
> mlx4_dev *dev, int slave,
> + int index,
> + enum res_cq_states
> state) {
> struct mlx4_priv *priv = mlx4_priv(dev);
> struct mlx4_resource_tracker *tracker =
> &priv->mfunc.master.res_tracker; @@ -1167,14 +1169,14 @@ static int
> srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> r->com.from_state = r->com.state; r->com.to_state = state;
> r->com.state = RES_SRQ_BUSY;
> - if (srq)
> - *srq = r;
> + } else {
> + r = ERR_PTR(err);
> }
> }
>
> spin_unlock_irq(mlx4_tlock(dev));
>
> - return err;
> + return r;
> }
>
> static void res_abort_move(struct mlx4_dev *dev, int slave,
> @@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev,
> int slave, struct res_cq *cq;
> struct res_mtt *mtt;
>
> - err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
> - if (err)
> - return err;
> + cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
> + if (IS_ERR(cq))
> + return PTR_ERR(cq);
> err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
> if (err)
> goto out_move;
> @@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev,
> int slave, int cqn = vhcr->in_modifier;
> struct res_cq *cq;
>
> - err = cq_res_start_move_to(dev, slave, cqn,
> RES_CQ_ALLOCATED, &cq);
> - if (err)
> - return err;
> + cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
> + if (IS_ERR(cq))
> + return PTR_ERR(cq);
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto out_move;
> @@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev
> *dev, int slave, if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) &
> 0xffffff)) return -EINVAL;
>
> - err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW,
> &srq);
> - if (err)
> - return err;
> + srq = srq_res_start_move_to(dev, slave, srqn,
> RES_SRQ_ALLOCATED);
> + if (IS_ERR(srq))
> + return PTR_ERR(srq);
> err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
> if (err)
> goto ex_abort;
> @@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev
> *dev, int slave, int srqn = vhcr->in_modifier;
> struct res_srq *srq;
>
> - err = srq_res_start_move_to(dev, slave, srqn,
> RES_SRQ_ALLOCATED, &srq);
> - if (err)
> - return err;
> + srq = srq_res_start_move_to(dev, slave, srqn,
> RES_SRQ_ALLOCATED);
> + if (IS_ERR(srq))
> + return PTR_ERR(srq);
> err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
> if (err)
> goto ex_abort;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4]
2013-10-01 15:17 ` Jack Morgenstein
@ 2013-10-01 15:25 ` Prarit Bhargava
2013-10-01 16:57 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Prarit Bhargava @ 2013-10-01 15:25 UTC (permalink / raw)
To: Jack Morgenstein; +Cc: netdev, dledford, amirv, davem, ogerlitz
On 10/01/2013 11:17 AM, Jack Morgenstein wrote:
> Since we are in the middle of submitting patches which touch the file
> "resource_tracker.c", I would really like to hold off on these warning
> fixes for a bit, and I'll handle the changes for all the functions at
> once to conform (correctly!) to the format suggested by Dave Miller.
>
> -Jack
Hey Jack, np. Thanks for looking.
P.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4]
2013-10-01 15:25 ` Prarit Bhargava
@ 2013-10-01 16:57 ` David Miller
0 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2013-10-01 16:57 UTC (permalink / raw)
To: prarit; +Cc: jackm, netdev, dledford, amirv, ogerlitz
From: Prarit Bhargava <prarit@redhat.com>
Date: Tue, 01 Oct 2013 11:25:28 -0400
>
>
> On 10/01/2013 11:17 AM, Jack Morgenstein wrote:
>> Since we are in the middle of submitting patches which touch the file
>> "resource_tracker.c", I would really like to hold off on these warning
>> fixes for a bit, and I'll handle the changes for all the functions at
>> once to conform (correctly!) to the format suggested by Dave Miller.
>>
>> -Jack
>
> Hey Jack, np. Thanks for looking.
Please repost both patches when you've sorted this out, thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-01 16:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-01 13:24 [PATCH 1/2] net, vxlan Fix compile warning [v4] Prarit Bhargava
2013-10-01 13:24 ` [PATCH 2/2] net, mellanox mlx4 Fix compile warnings [v4] Prarit Bhargava
2013-10-01 15:17 ` Jack Morgenstein
2013-10-01 15:25 ` Prarit Bhargava
2013-10-01 16:57 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).