All of this sounds reasonable to me. Thanks for writing this up. Ethan
On Thu, Jan 12, 2012 at 15:47, Ben Pfaff <b...@nicira.com> wrote: > On Thu, Jan 12, 2012 at 03:17:56PM -0800, Ethan Jackson wrote: >> My compiler complains about this patch: >> >> ofproto/ofproto.c: In function ?ofoperation_complete?: >> ofproto/ofproto.c:3157:5: error: comparison of unsigned expression >= >> 0 is always true [-Werror=type-limits] > > I guess that's the line "assert(error >= 0);"? OK, I deleted it now. > >> In ofperr_encode_msg__() the following line: >> pair = &domain->errors[MIN(error - OFPERR_OFS, OFPERR_N_ERRORS)]; >> >> I don't think you need the MIN because you know at this point that >> 'error' is valid and therefore error - OFPERR_OFS is less than >> OFPERR_N_ERRORS. > > That makes sense. Unfortunately GCC bug #43949 > (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43949) causes me to get a > bunch of bogus warnings when I do that. I think that's why I put it > there in the first place. > > I changed this to: > ofs = error - OFPERR_OFS; > pair = &domain->errors[ofs]; > which for whatever reason avoids the bogus warning. > >> Also in the same function you have a cast without a space following it. > > Thanks, I fixed this. > > I'll do a little more testing and push. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
