Re: [PATCH] net: make getname() functions return length rather than use int* parameter
On 02/12/2018 06:47 PM, David Miller wrote: From: Denys VlasenkoDate: 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
From: Denys VlasenkoDate: 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
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 VlasenkoCC: 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 @@