On Thu, Feb 19, 2015 at 12:41:48PM -0500, Russell Bryant wrote:
> On 02/19/2015 12:19 PM, Ben Pfaff wrote:
> > 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.
> >
>
> Fair enough. FWIW, here's the impact to the code if it were added.
> It's not bad and actually drops a cast.
You're right, this is an improvement. May I have a Signed-off-by to
commit this? A Signed-off-by has the following form and meaning, as
described in CONTRIBUTING.md. (This is the same form and meaning as
used in Linux kernel development.)
Signed-off-by: Author Name <author.name@email.address...>
Informally, this indicates that Author Name is the author or
submitter of a patch and has the authority to submit it under
the terms of the license. The formal meaning is to agree to
the Developer's Certificate of Origin (see below).
If the author and submitter are different, each must sign off.
If the patch has more than one author, all must sign off.
Signed-off-by: Author Name <author.name@email.address...>
Signed-off-by: Submitter Name <submitter.name@email.address...>
Developer's Certificate of Origin
---------------------------------
To help track the author of a patch as well as the submission chain,
and be clear that the developer has authority to submit a patch for
inclusion in openvswitch please sign off your work. The sign off
certifies the following:
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
