Hi,
> I'm not sure I'm convinced. This is quite a lot of extra code for
> an "add a few extra fields to a warning" enhancement - especially
> given that those that are already confused by the warning today
> will be more confused by "more numbers"...
>
I assume that people are confused by the fact of seeing warning,
less by provided numbers.
The patch adds a few more lines of code, but those are needed to provide
more information to fix/workaroud the issue. With
proto saddr:sport -> daddr:dport
it should be easy to figure out what app generates problematic traffic. Also
warning now tells user what to do - I think it is much more useful
comparison
to "someting is wrong" - style message what we have now. There are quite
many posts on internet asking "hey I have this warning, what is that, how
to fix"
and I think this patch helps address some of those.
> /* make sure we got whole IP header */
> > - if (BLEN(buf) < ((int) sizeof(struct openvpn_iphdr) +
> ip_hdr_offset))
> > + if (BLEN(buf) < ((int)sizeof(struct openvpn_iphdr) +
> ip_hdr_offset))
>
> In addition, these changes look funny. The existing code style was
> produced
> by the scripted big code cleanup for 2.4, so there should never be a
> reason to change the style in a patch for "master".
The "whitespace after cast" thing is inconsistent at the moment, have a
look at proto.c:
51 if (BLEN(buf) < (int) sizeof(struct openvpn_iphdr))
60 if (BLEN(buf) < (int)(sizeof(struct openvpn_ethhdr)
both of those lines were touched in "The Greate Reformatting - first phase"
commit.
Let's discuss this at the next meeting.
--
-Lev
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
