| Message ID | 20171108185743.3803-1-rosenp@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [LEDE-DEV] umdns: Replace unnecessary memset calls with {}. | expand |
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;
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
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
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
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;
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(-)