On Thu, Feb 19, 2015 at 10:45:21AM -0500, Russell Bryant wrote:
> On 02/19/2015 10:34 AM, Russell Bryant wrote:
> > On 02/19/2015 03:12 AM, Ben Pfaff wrote:
> >> The lower layers count errors but until now nothing actually reported them.
> >>
> >> Found by inspection.
> >>
> >> Signed-off-by: Ben Pfaff <b...@nicira.com>
> >> ---
> >>  vswitchd/bridge.c | 8 +++++---
> >>  1 file changed, 5 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> >> index 297c0dd..a143be1 100644
> >> --- a/vswitchd/bridge.c
> >> +++ b/vswitchd/bridge.c
> >> @@ -1,4 +1,4 @@
> >> -/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> >> +/* Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, 
> >> Inc.
> >>   *
> >>   * Licensed under the Apache License, Version 2.0 (the "License");
> >>   * you may not use this file except in compliance with the License.
> >> @@ -2489,8 +2489,8 @@ port_refresh_rstp_status(struct port *port)
> >>      struct ofproto *ofproto = port->bridge->ofproto;
> >>      struct iface *iface;
> >>      struct ofproto_port_rstp_status status;
> >> -    char *keys[3];
> >> -    int64_t int_values[3];
> >> +    char *keys[4];
> > 
> > Based on the code below, it looks like it would be nice to make this
> > "const char *keys[4];".
> > 
> > If that gets changed, it's passed to automatically generated code where
> > the parameter is not const.  It's the 2nd parameter here:
> > 
> > ovsrec_port_set_statistics(const struct ovsrec_port *row, char
> > **key_statistics, const int64_t *value_statistics, size_t n_statistics)
> > 
> > The second parameter is explicitly excluded from being made const, but
> > it's not clear why.  Is there some C detail I'm forgetting, or does this
> > seem safe?
> > 
> > The following change results in the 2nd parameter above being made const
> > and seems to still build without any warnings:
> 
> Nevermind ... of course there's a bunch of warnings.  I just didn't set
> -Werror.
> 
> It looks like the below change wouldn't be desired, but maybe adding
> const to just "char **" would be OK.  Of course, it's not important
> anyway ...

"const" has really weird semantics in cases with double or multiple
pointers in C (C++ is actually more sane here) so I'm in the habit of
leaving off const in such cases.  Otherwise you end up with casts in
places where it seems like you shouldn't need them.  That's why I tend
not to use them.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to