On Wed, Sep 11, 2019 at 11:30:38AM +0300, Dan Carpenter wrote:

On Wed, Sep 11, 2019 at 09:30:47AM +0800, maowenan wrote:

quoted

On 2019/9/11 3:22, Dan Carpenter wrote:

quoted

On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:

quoted

On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:

quoted

There are more parentheses in if clause when call sctp_get_port_local
in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
do cleanup.

Signed-off-by: Mao Wenan <redacted>
---
 net/sctp/socket.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 9d1f83b10c0a..766b68b55ebe 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c

@@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	 * detection.
 	 */
 	addr->v4.sin_port = htons(snum);
-	if ((ret = sctp_get_port_local(sk, addr))) {
+	if (sctp_get_port_local(sk, addr))
 		return -EADDRINUSE;

sctp_get_port_local() returns a long which is either 0,1 or a pointer
casted to long.  It's not documented what it means and neither of the
callers use the return since commit 62208f12451f ("net: sctp: simplify
sctp_get_port").

Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
because before the code assumed that a pointer casted to an int was the
same as a pointer casted to a long.

commit 4e54064e0a13 treated non-zero return value as unexpected, so the current
cleanup is ok?

Yeah.  It's fine, I was just confused why we weren't preserving the
error code and then I saw that we didn't return errors at all and got
confused.

But please lets seize the moment and do the change Dean suggested.
This was the last place saving this return value somewhere. It makes
sense to cleanup sctp_get_port_local() now and remove that masked
pointer return.

Then you may also cleanup:
socket.c:       return !!sctp_get_port_local(sk, &addr);
as it will be a direct map.

  Marcelo