On Fri, Jan 05, 2018 at 04:42:00AM +0800, Ben Pfaff wrote: > On Fri, Dec 08, 2017 at 10:04:22PM +0800, Yi Yang wrote: > > Signed-off-by: Yi Yang <[email protected]> > > This fails to build with Clang (and, I would guess, MSVC): > ../lib/odp-execute.c:497:21: error: fields must have a constant size: > 'variable length array in structure' extension will never be supported > > "sparse" issues some warnings. This one is probably sensible: > ../lib/odp-util.c:1884:32: warning: incorrect type in argument 1 > (different base types) > ../lib/odp-util.c:1884:32: expected unsigned int [unsigned] [usertype] > x > ../lib/odp-util.c:1884:32: got restricted ovs_be32 [assigned] > [usertype] spi > > These I don't understand. > https://marc.info/?l=linux-sparse&m=110218288411207 suggests that they > might be a sparse bug: > > ../lib/odp-util.c:7123:32: warning: crazy programmer > ../lib/odp-util.c:7123:43: warning: crazy programmer > ../lib/odp-util.c:7123:22: warning: crazy programmer > > In odp_execute_actions(), this looks bogus: there is nothing to > guarantee that 'buffer' is properly aligned for struct nsh_hdr. > + uint8_t buffer[NSH_HDR_MAX_LEN]; > + struct nsh_hdr *nsh_hdr = ALIGNED_CAST(struct nsh_hdr *, buffer); > > Similarly in format_odp_action(). > > In nsh_key_to_attr(), can this: > + for (int i = 0; i < 4; i++) { > + md1.context[i] = nsh->context[i]; > + } > + nl_msg_put_unspec(buf, OVS_NSH_KEY_ATTR_MD1, &md1, sizeof md1); > be written as just this? > + nl_msg_put_unspec(buf, OVS_NSH_KEY_ATTR_MD1, nsh->context, > + sizeof nsh->context); > and similarly in a second place. > > In parse_odp_push_nsh_action(), the idea of xmalloc()'ing a stub is > weird. A stub should be a local array. There are many examples in the > tree. > > Please don't check a pointer for null before calling free(): > + if (metadata != NULL) { > + free(metadata); > }
Thanks, Ben, I have posted v7 https://mail.openvswitch.org/pipermail/ovs-dev/2018-January/342698.html to fix the above issues. By the way, what command do you run to do static code analysis by sparse? "make clang-analyze" will have too much warnings. I can't get sparse warnings, it will be better if I can run it locally. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
