[LEDE-DEV] umdns: Replace unnecessary memset calls with {}.

Message ID 20171108185743.3803-1-rosenp@gmail.com
State Rejected
Headers show
Series
  • [LEDE-DEV] umdns: Replace unnecessary memset calls with {}.
Related show

Commit Message

Rosen Penev Nov. 8, 2017, 6:57 p.m.
Less verbose.

Signed-off-by: Rosen Penev <rosenp@gmail.com>

v2: some of those memset calls are needed. Also replace { 0 } with {}.
---
 interface.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

Arjen de Korte Nov. 8, 2017, 8:17 p.m. | #1
Citeren Rosen Penev <rosenp@gmail.com>:

> Less verbose.

And uses a GCC extension which makes it less portable. ISO C forbids  
empty initializer braces [1]. See for yourself by adding the -pedantic  
flag to your CFLAGS. The correct way to initialize to all-zeros is  
therefore { 0 }.

[1] ISO/IEC 9899:201x, paragraph 6.7.9 Initialization, clause 21

"If there are fewer initializers in a brace-enclosed list than there  
are elements or members
of an aggregate, or fewer characters in a string literal used to  
initialize an array of known
size  than  there  are  elements  in  the  array,  the  remainder  of   
the  aggregate  shall  be
initialized implicitly the same as objects that have static storage duration."

> Signed-off-by: Rosen Penev <rosenp@gmail.com>
>
> v2: some of those memset calls are needed. Also replace { 0 } with {}.
> ---
>  interface.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/interface.c b/interface.c
> index 7f814d2..deabcbb 100644
> --- a/interface.c
> +++ b/interface.c
> @@ -186,7 +186,7 @@ read_socket4(struct uloop_fd *u, unsigned int events)
>  	struct iovec iov[1];
>  	char cmsg[CMSG_SPACE(sizeof(struct in_pktinfo)) +  
> CMSG_SPACE(sizeof(int)) + 1];
>  	struct cmsghdr *cmsgptr;
> -	struct msghdr msg;
> +	struct msghdr msg = {};
>  	socklen_t len;
>  	struct sockaddr_in from;
>  	int flags = 0, ifindex = -1;
> @@ -202,7 +202,6 @@ read_socket4(struct uloop_fd *u, unsigned int events)
>  	iov[0].iov_base = buffer;
>  	iov[0].iov_len = sizeof(buffer);
>
> -	memset(&msg, 0, sizeof(msg));
>  	msg.msg_name = (struct sockaddr *) &from;
>  	msg.msg_namelen = sizeof(struct sockaddr_in);
>  	msg.msg_iov = iov;
> @@ -260,7 +259,7 @@ read_socket6(struct uloop_fd *u, unsigned int events)
>  	struct iovec iov[1];
>  	char cmsg6[CMSG_SPACE(sizeof(struct in6_pktinfo)) +  
> CMSG_SPACE(sizeof(int)) + 1];
>  	struct cmsghdr *cmsgptr;
> -	struct msghdr msg;
> +	struct msghdr msg = {};
>  	socklen_t len;
>  	struct sockaddr_in6 from;
>  	int flags = 0, ifindex = -1;
> @@ -276,7 +275,6 @@ read_socket6(struct uloop_fd *u, unsigned int events)
>  	iov[0].iov_base = buffer;
>  	iov[0].iov_len = sizeof(buffer);
>
> -	memset(&msg, 0, sizeof(msg));
>  	msg.msg_name = (struct sockaddr *) &from;
>  	msg.msg_namelen = sizeof(struct sockaddr_in6);
>  	msg.msg_iov = iov;
> @@ -327,17 +325,16 @@ read_socket6(struct uloop_fd *u, unsigned int events)
>  static int
>  interface_mcast_setup4(struct interface *iface)
>  {
> -	struct ip_mreqn mreq;
> +	struct ip_mreqn mreq = {};
>  	uint8_t ttl = 255;
>  	int no = 0;
> -	struct sockaddr_in sa = { 0 };
> +	struct sockaddr_in sa = {};
>  	int fd = iface->fd.fd;
>
>  	sa.sin_family = AF_INET;
>  	sa.sin_port = htons(MCAST_PORT);
>  	inet_pton(AF_INET, MCAST_ADDR, &sa.sin_addr);
>
> -	memset(&mreq, 0, sizeof(mreq));
>  	mreq.imr_address.s_addr = iface->v4_addr.s_addr;
>  	mreq.imr_multiaddr = sa.sin_addr;
>  	mreq.imr_ifindex = iface->ifindex;
> @@ -368,17 +365,16 @@ interface_mcast_setup4(struct interface *iface)
>  static int
>  interface_socket_setup6(struct interface *iface)
>  {
> -	struct ipv6_mreq mreq;
> +	struct ipv6_mreq mreq = {};
>  	int ttl = 255;
>  	int no = 0;
> -	struct sockaddr_in6 sa = { 0 };
> +	struct sockaddr_in6 sa = {};
>  	int fd = iface->fd.fd;
>
>  	sa.sin6_family = AF_INET6;
>  	sa.sin6_port = htons(MCAST_PORT);
>  	inet_pton(AF_INET6, MCAST_ADDR6, &sa.sin6_addr);
>
> -	memset(&mreq, 0, sizeof(mreq));
>  	mreq.ipv6mr_multiaddr = sa.sin6_addr;
>  	mreq.ipv6mr_interface = iface->ifindex;
Karl Palsson Nov. 8, 2017, 8:34 p.m. | #2
Rosen Penev <rosenp@gmail.com> wrote:
> Less verbose.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> 
> v2: some of those memset calls are needed. Also replace { 0 }
> with {}.

cmon, that's just unnecessary. It's one thing to prefer using gnu
{} instead of ISO C {0} but replacing ISO standard C form with a
gnu form, just because? What on earth for?

Sincerely,
Karl Palsson
Rosen Penev Nov. 13, 2017, 2:49 a.m. | #3
On Wed, 2017-11-08 at 21:17 +0100, Arjen de Korte wrote:
> Citeren Rosen Penev <rosenp@gmail.com>:
> 
> > Less verbose.
> 
> And uses a GCC extension which makes it less portable. ISO C
> forbids  
> empty initializer braces [1]. See for yourself by adding the
> -pedantic  
> flag to your CFLAGS. The correct way to initialize to all-zeros is  
> therefore { 0 }.
> 
> [1] ISO/IEC 9899:201x, paragraph 6.7.9 Initialization, clause 21
> 
> "If there are fewer initializers in a brace-enclosed list than
> there  
> are elements or members
> of an aggregate, or fewer characters in a string literal used to  
> initialize an array of known
> size  than  there  are  elements  in  the  array,  the  remainder  of
>    
> the  aggregate  shall  be
> initialized implicitly the same as objects that have static storage
> duration."

I decided to test this with the following program.


#include <stdio.h>
#include <stdint.h>
#include <string.h>

int main()
{
        struct k {
                int h;
                int t;
        };

	struct k z = {5};
        printf("%d", z.t);

        return 0;
}

0 was printed instead of 5. 
> 
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > 
> > v2: some of those memset calls are needed. Also replace { 0 } with
> > {}.
> > ---
> >  interface.c | 16 ++++++----------
> >  1 file changed, 6 insertions(+), 10 deletions(-)
> > 
> > diff --git a/interface.c b/interface.c
> > index 7f814d2..deabcbb 100644
> > --- a/interface.c
> > +++ b/interface.c
> > @@ -186,7 +186,7 @@ read_socket4(struct uloop_fd *u, unsigned int
> > events)
> >  	struct iovec iov[1];
> >  	char cmsg[CMSG_SPACE(sizeof(struct in_pktinfo)) +  
> > CMSG_SPACE(sizeof(int)) + 1];
> >  	struct cmsghdr *cmsgptr;
> > -	struct msghdr msg;
> > +	struct msghdr msg = {};
> >  	socklen_t len;
> >  	struct sockaddr_in from;
> >  	int flags = 0, ifindex = -1;
> > @@ -202,7 +202,6 @@ read_socket4(struct uloop_fd *u, unsigned int
> > events)
> >  	iov[0].iov_base = buffer;
> >  	iov[0].iov_len = sizeof(buffer);
> > 
> > -	memset(&msg, 0, sizeof(msg));
> >  	msg.msg_name = (struct sockaddr *) &from;
> >  	msg.msg_namelen = sizeof(struct sockaddr_in);
> >  	msg.msg_iov = iov;
> > @@ -260,7 +259,7 @@ read_socket6(struct uloop_fd *u, unsigned int
> > events)
> >  	struct iovec iov[1];
> >  	char cmsg6[CMSG_SPACE(sizeof(struct in6_pktinfo)) +  
> > CMSG_SPACE(sizeof(int)) + 1];
> >  	struct cmsghdr *cmsgptr;
> > -	struct msghdr msg;
> > +	struct msghdr msg = {};
> >  	socklen_t len;
> >  	struct sockaddr_in6 from;
> >  	int flags = 0, ifindex = -1;
> > @@ -276,7 +275,6 @@ read_socket6(struct uloop_fd *u, unsigned int
> > events)
> >  	iov[0].iov_base = buffer;
> >  	iov[0].iov_len = sizeof(buffer);
> > 
> > -	memset(&msg, 0, sizeof(msg));
> >  	msg.msg_name = (struct sockaddr *) &from;
> >  	msg.msg_namelen = sizeof(struct sockaddr_in6);
> >  	msg.msg_iov = iov;
> > @@ -327,17 +325,16 @@ read_socket6(struct uloop_fd *u, unsigned int
> > events)
> >  static int
> >  interface_mcast_setup4(struct interface *iface)
> >  {
> > -	struct ip_mreqn mreq;
> > +	struct ip_mreqn mreq = {};
> >  	uint8_t ttl = 255;
> >  	int no = 0;
> > -	struct sockaddr_in sa = { 0 };
> > +	struct sockaddr_in sa = {};
> >  	int fd = iface->fd.fd;
> > 
> >  	sa.sin_family = AF_INET;
> >  	sa.sin_port = htons(MCAST_PORT);
> >  	inet_pton(AF_INET, MCAST_ADDR, &sa.sin_addr);
> > 
> > -	memset(&mreq, 0, sizeof(mreq));
> >  	mreq.imr_address.s_addr = iface->v4_addr.s_addr;
> >  	mreq.imr_multiaddr = sa.sin_addr;
> >  	mreq.imr_ifindex = iface->ifindex;
> > @@ -368,17 +365,16 @@ interface_mcast_setup4(struct interface
> > *iface)
> >  static int
> >  interface_socket_setup6(struct interface *iface)
> >  {
> > -	struct ipv6_mreq mreq;
> > +	struct ipv6_mreq mreq = {};
> >  	int ttl = 255;
> >  	int no = 0;
> > -	struct sockaddr_in6 sa = { 0 };
> > +	struct sockaddr_in6 sa = {};
> >  	int fd = iface->fd.fd;
> > 
> >  	sa.sin6_family = AF_INET6;
> >  	sa.sin6_port = htons(MCAST_PORT);
> >  	inet_pton(AF_INET6, MCAST_ADDR6, &sa.sin6_addr);
> > 
> > -	memset(&mreq, 0, sizeof(mreq));
> >  	mreq.ipv6mr_multiaddr = sa.sin6_addr;
> >  	mreq.ipv6mr_interface = iface->ifindex;
> 
> 
> 
> 
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
Eric Luehrsen Nov. 13, 2017, 4:55 a.m. | #4
On 11/12/2017 09:49 PM, rosenp@gmail.com wrote:
> On Wed, 2017-11-08 at 21:17 +0100, Arjen de Korte wrote:
>> Citeren Rosen Penev <rosenp@gmail.com>:
>>
>>> Less verbose.
>> And uses a GCC extension which makes it less portable. ISO C
>> forbids
>> empty initializer braces [1]. See for yourself by adding the
>> -pedantic
>> flag to your CFLAGS. The correct way to initialize to all-zeros is
>> therefore { 0 }.
>>
>> [1] ISO/IEC 9899:201x, paragraph 6.7.9 Initialization, clause 21
>>
>> "If there are fewer initializers in a brace-enclosed list than
>> there
>> are elements or members
>> of an aggregate, or fewer characters in a string literal used to
>> initialize an array of known
>> size  than  there  are  elements  in  the  array,  the  remainder  of
>>     
>> the  aggregate  shall  be
>> initialized implicitly the same as objects that have static storage
>> duration."
> I decided to test this with the following program.
>
>
> #include <stdio.h>
> #include <stdint.h>
> #include <string.h>
>
> int main()
> {
>          struct k {
>                  int h;
>                  int t;
>          };
>
> 	struct k z = {5};
>          printf("%d", z.t);
>
>          return 0;
> }
> ;
> 0 was printed instead of 5.
This is what should happen. It doesn't repeat a pattern. To rephrase 
ISO-C standard. If the implied initialization of COMMON or weak 
definitions is 0, then partial initialization will initialize forgotten 
elements to 0. As a consequence one quality check done in some 
organizations is expect: {0} only, maybe require curls representing 
nested depth, or all elements explicitly initialized, else any other is 
rejected.

- Eric

Patch

diff --git a/interface.c b/interface.c
index 7f814d2..deabcbb 100644
--- a/interface.c
+++ b/interface.c
@@ -186,7 +186,7 @@  read_socket4(struct uloop_fd *u, unsigned int events)
 	struct iovec iov[1];
 	char cmsg[CMSG_SPACE(sizeof(struct in_pktinfo)) + CMSG_SPACE(sizeof(int)) + 1];
 	struct cmsghdr *cmsgptr;
-	struct msghdr msg;
+	struct msghdr msg = {};
 	socklen_t len;
 	struct sockaddr_in from;
 	int flags = 0, ifindex = -1;
@@ -202,7 +202,6 @@  read_socket4(struct uloop_fd *u, unsigned int events)
 	iov[0].iov_base = buffer;
 	iov[0].iov_len = sizeof(buffer);
 
-	memset(&msg, 0, sizeof(msg));
 	msg.msg_name = (struct sockaddr *) &from;
 	msg.msg_namelen = sizeof(struct sockaddr_in);
 	msg.msg_iov = iov;
@@ -260,7 +259,7 @@  read_socket6(struct uloop_fd *u, unsigned int events)
 	struct iovec iov[1];
 	char cmsg6[CMSG_SPACE(sizeof(struct in6_pktinfo)) + CMSG_SPACE(sizeof(int)) + 1];
 	struct cmsghdr *cmsgptr;
-	struct msghdr msg;
+	struct msghdr msg = {};
 	socklen_t len;
 	struct sockaddr_in6 from;
 	int flags = 0, ifindex = -1;
@@ -276,7 +275,6 @@  read_socket6(struct uloop_fd *u, unsigned int events)
 	iov[0].iov_base = buffer;
 	iov[0].iov_len = sizeof(buffer);
 
-	memset(&msg, 0, sizeof(msg));
 	msg.msg_name = (struct sockaddr *) &from;
 	msg.msg_namelen = sizeof(struct sockaddr_in6);
 	msg.msg_iov = iov;
@@ -327,17 +325,16 @@  read_socket6(struct uloop_fd *u, unsigned int events)
 static int
 interface_mcast_setup4(struct interface *iface)
 {
-	struct ip_mreqn mreq;
+	struct ip_mreqn mreq = {};
 	uint8_t ttl = 255;
 	int no = 0;
-	struct sockaddr_in sa = { 0 };
+	struct sockaddr_in sa = {};
 	int fd = iface->fd.fd;
 
 	sa.sin_family = AF_INET;
 	sa.sin_port = htons(MCAST_PORT);
 	inet_pton(AF_INET, MCAST_ADDR, &sa.sin_addr);
 
-	memset(&mreq, 0, sizeof(mreq));
 	mreq.imr_address.s_addr = iface->v4_addr.s_addr;
 	mreq.imr_multiaddr = sa.sin_addr;
 	mreq.imr_ifindex = iface->ifindex;
@@ -368,17 +365,16 @@  interface_mcast_setup4(struct interface *iface)
 static int
 interface_socket_setup6(struct interface *iface)
 {
-	struct ipv6_mreq mreq;
+	struct ipv6_mreq mreq = {};
 	int ttl = 255;
 	int no = 0;
-	struct sockaddr_in6 sa = { 0 };
+	struct sockaddr_in6 sa = {};
 	int fd = iface->fd.fd;
 
 	sa.sin6_family = AF_INET6;
 	sa.sin6_port = htons(MCAST_PORT);
 	inet_pton(AF_INET6, MCAST_ADDR6, &sa.sin6_addr);
 
-	memset(&mreq, 0, sizeof(mreq));
 	mreq.ipv6mr_multiaddr = sa.sin6_addr;
 	mreq.ipv6mr_interface = iface->ifindex;