Thanks for the review, pushed with this error message line (it fit to one line 
better :-):

        VLOG_WARN("Negative value %s is not a valid port number.", s);

Jarno

On Apr 17, 2014, at 11:11 PM, Justin Pettit <jpet...@nicira.com> wrote:

> It might be useful to print the port string that was handed to it, since the 
> other errors in that function do that.  Something like:
> 
>   VLOG_WARN("port %s is a negative value, which is not a valid port number", 
> s);
> 
> Assuming that seems reasonable to you and Zoltan thinks it would prevent the 
> issue he saw:
> 
> Acked-by: Justin Pettit <jpet...@nicira.com>
> 
> --Justin
> 
> 
> On April 17, 2014 at 6:37:42 PM, Jarno Rajahalme (jrajaha...@nicira.com) 
> wrote:
>> Commit 33ab38d9 (meta-flow: Simplify mf_from_ofp_port_string())
>> inadvertently removed a check for minus sign at the beginning of a
>> port number string introduced by commit 05dddba (meta-flow: Don't
>> allow negative port numbers). This check is still needed, so put it
>> back, but to ofputil_port_from_string() this time.
>> 
>> Signed-off-by: Jarno Rajahalme  
>> ---
>> lib/ofp-util.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
>> index af2d853..a990f46 100644
>> --- a/lib/ofp-util.c
>> +++ b/lib/ofp-util.c
>> @@ -5394,8 +5394,12 @@ ofputil_port_to_ofp11(ofp_port_t ofp10_port)
>> bool
>> ofputil_port_from_string(const char *s, ofp_port_t *portp)
>> {
>> - uint32_t port32;
>> + unsigned int port32; /* int is at least 32 bits wide. */
>> 
>> + if (*s == '-') {
>> + VLOG_WARN("Negative values are not supported as port numbers.");
>> + return false;
>> + }
>> *portp = 0;
>> if (str_to_uint(s, 10, &port32)) {
>> if (port32 < ofp_to_u16(OFPP_MAX)) {
>> --
>> 1.7.10.4
>> 
>> _______________________________________________
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to