Hi Tim, On Sun, Jan 21, 2018 at 01:24:06PM +0100, Tim Düsterhus wrote: > Jarno, > > Am 15.01.2018 um 14:28 schrieb Jarno Huuskonen: > > I'll try to test later (might be couple of days). > > Did you find time, yet? > > > I'll get a compilation error on centos7: > > src/sample.c: In function 'sample_conv_ipmask': > > src/sample.c:1623:3: error: 'for' loop initial declarations are only > > allowed in C99 mode > > for (size_t i = 0; i < 16; i++) > > ^ > > src/sample.c:1623:3: note: use option -std=c99 or -std=gnu99 to compile > > your code > > > > So maybe move size_t i outside of loop or unroll loop, something like
Yes, please avoid declaring variables inside the for() loop or in the middle of the code, over time they make code maintenance more painful because when you look for the type of a variable you have to scrutinize the code backwards till the beginning of the function instead of quickly glancing at the beginning of each block. > > + *(uint32_t*)&smp->data.u.ipv6.s6_addr[0] &= > > *(uint32_t*)&arg_p[1].data.ipv6.s6_addr[0]; > > + *(uint32_t*)&smp->data.u.ipv6.s6_addr[4] &= > > *(uint32_t*)&arg_p[1].data.ipv6.s6_addr[4]; > > + *(uint32_t*)&smp->data.u.ipv6.s6_addr[8] &= > > *(uint32_t*)&arg_p[1].data.ipv6.s6_addr[8]; > > + *(uint32_t*)&smp->data.u.ipv6.s6_addr[12] &= > > *(uint32_t*)&arg_p[1].data.ipv6.s6_addr[12]; > > > > Thinking about this for some time: I am not sure whether casting a char > pointer to a uint32_t pointer is safe from a language lawyer perspective > in all cases. I certainly can update the patch when Willy tells me his > preference (just move the declaration up or perform the casting). There's no problem here because these ones are word-aligned anyway. There are other such occurrences elsewhere in the code, so don't worry for this one. >From what I've seen till now your patches look fine but like you I'm interested in Jarno's feedback ;-) Cheers, Willy
