Re: [PATCH] net: make getname() functions return length rather than use int* parameter

[Denys Vlasenko]

On 02/12/2018 06:47 PM, David Miller wrote:

From: Denys Vlasenko 
Date: Mon, 12 Feb 2018 15:15:18 +0100


Before:
All these functions either return a negative error indicator,
or store length of sockaddr into "int *socklen" parameter
and return zero on success.

"int *socklen" parameter is awkward. For example, if caller does not
care, it still needs to provide on-stack storage for the value
it does not need.

None of the many FOO_getname() functions of various protocols
ever used old value of *socklen. They always just overwrite it.

This change drops this parameter, and makes all these functions, on success,
return length of sockaddr. It's always >= 0 and can be differentiated
from an error.

Tests in callers are changed from "if (err)" to "if (err < 0)", where needed.

rpc_sockname() lost "int buflen" parameter, since its only use was
to be passed to kernel_getsockname() as  and subsequently
not used in any way.

Userspace API is not changed.

 textdata bss  dec hex filename
30108430 2633624  873672 33615726 200ef6e vmlinux.before.o
30108109 2633612  873672 33615393 200ee21 vmlinux.o

Signed-off-by: Denys Vlasenko 


Please do an allmodconfig build, there are still some conversions you
missed:

security/tomoyo/network.c: In function ‘tomoyo_socket_listen_permission’:
security/tomoyo/network.c:658:19: warning: passing argument 3 of 
‘sock->ops->getname’ makes integer from pointer without a cast 
[-Wint-conversion]
 , _len, 0);
^
security/tomoyo/network.c:658:19: note: expected ‘int’ but argument is of type 
‘int *’
security/tomoyo/network.c:657:21: error: too many arguments to function 
‘sock->ops->getname’
const int error = sock->ops->getname(sock, (struct sockaddr *)
  ^~~~
fs/dlm/lowcomms.c: In function ‘lowcomms_error_report’:
fs/dlm/lowcomms.c:495:6: error: too many arguments to function 
‘kernel_getpeername’
   kernel_getpeername(con->sock, (struct sockaddr *), )) {
   ^~
fs/dlm/lowcomms.c: In function ‘tcp_accept_from_sock’:
fs/dlm/lowcomms.c:761:7: warning: passing argument 3 of ‘newsock->ops->getname’ 
makes integer from pointer without a cast [-Wint-conversion]
, 2)) {
^
fs/dlm/lowcomms.c:761:7: note: expected ‘int’ but argument is of type ‘int *’
fs/dlm/lowcomms.c:760:6: error: too many arguments to function 
‘newsock->ops->getname’
   if (newsock->ops->getname(newsock, (struct sockaddr *),
   ^~~


Sorry. Will send updated patch.
 

Re: [PATCH] net: make getname() functions return length rather than use int* parameter

[David Miller]

From: Denys Vlasenko 
Date: Mon, 12 Feb 2018 15:15:18 +0100

> Before:
> All these functions either return a negative error indicator,
> or store length of sockaddr into "int *socklen" parameter
> and return zero on success.
> 
> "int *socklen" parameter is awkward. For example, if caller does not
> care, it still needs to provide on-stack storage for the value
> it does not need.
> 
> None of the many FOO_getname() functions of various protocols
> ever used old value of *socklen. They always just overwrite it.
> 
> This change drops this parameter, and makes all these functions, on success,
> return length of sockaddr. It's always >= 0 and can be differentiated
> from an error.
> 
> Tests in callers are changed from "if (err)" to "if (err < 0)", where needed.
> 
> rpc_sockname() lost "int buflen" parameter, since its only use was
> to be passed to kernel_getsockname() as  and subsequently
> not used in any way.
> 
> Userspace API is not changed.
> 
> textdata bss  dec hex filename
> 30108430 2633624  873672 33615726 200ef6e vmlinux.before.o
> 30108109 2633612  873672 33615393 200ee21 vmlinux.o
> 
> Signed-off-by: Denys Vlasenko 

Please do an allmodconfig build, there are still some conversions you
missed:

security/tomoyo/network.c: In function ‘tomoyo_socket_listen_permission’:
security/tomoyo/network.c:658:19: warning: passing argument 3 of 
‘sock->ops->getname’ makes integer from pointer without a cast 
[-Wint-conversion]
, _len, 0);
   ^
security/tomoyo/network.c:658:19: note: expected ‘int’ but argument is of type 
‘int *’
security/tomoyo/network.c:657:21: error: too many arguments to function 
‘sock->ops->getname’
   const int error = sock->ops->getname(sock, (struct sockaddr *)
 ^~~~
fs/dlm/lowcomms.c: In function ‘lowcomms_error_report’:
fs/dlm/lowcomms.c:495:6: error: too many arguments to function 
‘kernel_getpeername’
  kernel_getpeername(con->sock, (struct sockaddr *), )) {
  ^~
fs/dlm/lowcomms.c: In function ‘tcp_accept_from_sock’:
fs/dlm/lowcomms.c:761:7: warning: passing argument 3 of ‘newsock->ops->getname’ 
makes integer from pointer without a cast [-Wint-conversion]
   , 2)) {
   ^
fs/dlm/lowcomms.c:761:7: note: expected ‘int’ but argument is of type ‘int *’
fs/dlm/lowcomms.c:760:6: error: too many arguments to function 
‘newsock->ops->getname’
  if (newsock->ops->getname(newsock, (struct sockaddr *),
  ^~~

[PATCH] net: make getname() functions return length rather than use int* parameter

[Denys Vlasenko]

Before:
All these functions either return a negative error indicator,
or store length of sockaddr into "int *socklen" parameter
and return zero on success.

"int *socklen" parameter is awkward. For example, if caller does not
care, it still needs to provide on-stack storage for the value
it does not need.

None of the many FOO_getname() functions of various protocols
ever used old value of *socklen. They always just overwrite it.

This change drops this parameter, and makes all these functions, on success,
return length of sockaddr. It's always >= 0 and can be differentiated
from an error.

Tests in callers are changed from "if (err)" to "if (err < 0)", where needed.

rpc_sockname() lost "int buflen" parameter, since its only use was
to be passed to kernel_getsockname() as  and subsequently
not used in any way.

Userspace API is not changed.

textdata bss  dec hex filename
30108430 2633624  873672 33615726 200ef6e vmlinux.before.o
30108109 2633612  873672 33615393 200ee21 vmlinux.o

Signed-off-by: Denys Vlasenko 
CC: David S. Miller 
CC: linux-ker...@vger.kernel.org
CC: net...@vger.kernel.org
CC: linux-blueto...@vger.kernel.org
CC: linux-decnet-u...@lists.sourceforge.net
CC: linux-wireless@vger.kernel.org
CC: linux-r...@vger.kernel.org
CC: linux-s...@vger.kernel.org
CC: linux-...@vger.kernel.org
CC: linux-...@vger.kernel.org
---
 drivers/isdn/mISDN/socket.c|  5 ++---
 drivers/net/ppp/pppoe.c|  6 ++
 drivers/net/ppp/pptp.c |  6 ++
 drivers/scsi/iscsi_tcp.c   | 14 +++---
 drivers/soc/qcom/qmi_interface.c   |  3 +--
 drivers/staging/ipx/af_ipx.c   |  6 ++
 drivers/staging/irda/net/af_irda.c |  8 +++-
 include/linux/net.h|  8 +++-
 include/net/inet_common.h  |  2 +-
 include/net/ipv6.h |  2 +-
 include/net/sock.h |  2 +-
 net/appletalk/ddp.c|  5 ++---
 net/atm/pvc.c  |  5 ++---
 net/atm/svc.c  |  5 ++---
 net/ax25/af_ax25.c |  4 ++--
 net/bluetooth/hci_sock.c   |  4 ++--
 net/bluetooth/l2cap_sock.c |  5 ++---
 net/bluetooth/rfcomm/sock.c|  5 ++---
 net/bluetooth/sco.c|  5 ++---
 net/can/raw.c  |  6 ++
 net/core/sock.c|  5 +++--
 net/decnet/af_decnet.c |  6 ++
 net/ipv4/af_inet.c |  5 ++---
 net/ipv6/af_inet6.c|  5 ++---
 net/iucv/af_iucv.c |  5 ++---
 net/l2tp/l2tp_ip.c |  5 ++---
 net/l2tp/l2tp_ip6.c|  5 ++---
 net/l2tp/l2tp_ppp.c|  5 ++---
 net/llc/af_llc.c   |  5 ++---
 net/netlink/af_netlink.c   |  5 ++---
 net/netrom/af_netrom.c |  9 +
 net/nfc/llcp_sock.c|  5 ++---
 net/packet/af_packet.c | 10 --
 net/phonet/socket.c|  5 ++---
 net/qrtr/qrtr.c|  5 ++---
 net/rds/af_rds.c   |  5 ++---
 net/rds/tcp.c  |  7 ++-
 net/rose/af_rose.c |  5 ++---
 net/sctp/ipv6.c|  8 
 net/smc/af_smc.c   |  7 +++
 net/socket.c   | 35 +--
 net/sunrpc/clnt.c  |  6 +++---
 net/sunrpc/svcsock.c   | 13 -
 net/sunrpc/xprtsock.c  |  3 +--
 net/tipc/socket.c  |  5 ++---
 net/unix/af_unix.c | 10 +-
 net/vmw_vsock/af_vsock.c   |  4 ++--
 net/x25/af_x25.c   |  4 ++--
 48 files changed, 132 insertions(+), 171 deletions(-)

diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index c5603d1a07d6..1f8f489b4167 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -560,7 +560,7 @@ data_sock_bind(struct socket *sock, struct sockaddr *addr, 
int addr_len)
 
 static int
 data_sock_getname(struct socket *sock, struct sockaddr *addr,
- int *addr_len, int peer)
+ int peer)
 {
struct sockaddr_mISDN   *maddr = (struct sockaddr_mISDN *) addr;
struct sock *sk = sock->sk;
@@ -570,14 +570,13 @@ data_sock_getname(struct socket *sock, struct sockaddr 
*addr,
 
lock_sock(sk);
 
-   *addr_len = sizeof(*maddr);
maddr->family = AF_ISDN;
maddr->dev = _pms(sk)->dev->id;
maddr->channel = _pms(sk)->ch.nr;
maddr->sapi = _pms(sk)->ch.addr & 0xff;
maddr->tei = (_pms(sk)->ch.addr >> 8) & 0xff;
release_sock(sk);
-   return 0;
+   return sizeof(*maddr);
 }
 
 static const struct proto_ops data_sock_ops = {
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 5aa59f41bf8c..bd89d1c559ce 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@