On Fri, Nov 16, 2012 at 10:49:39AM -0800, Justin Pettit wrote:
> On Nov 16, 2012, at 9:54 AM, Ben Pfaff <b...@nicira.com> wrote:
>
> > On Fri, Nov 16, 2012 at 12:03:02AM -0800, Justin Pettit wrote:
> >> The kernel datapath automatically "up"s internal devices, but this
> >> wasn't happening for the userspace datapath. This change has the bridge
> >> module always "up" them.
> >>
> >> Signed-off-by: Justin Pettit <jpet...@nicira.com>
> >
> > Seems reasonable, thanks.
> >
> > The indentation here, on the second line, looks odd to me:
> > if ((port_cfg->vlan_mode && !strcmp(port_cfg->vlan_mode, "splinter"))
> > || iface_is_internal(iface_cfg, br->cfg)) {
> > I would write it as:
> > if ((port_cfg->vlan_mode && !strcmp(port_cfg->vlan_mode, "splinter"))
> > || iface_is_internal(iface_cfg, br->cfg)) {
>
> I believe that would line it up with the code block within the
> statement, which looks odd to me. In our CodingStyle, it seems like
> there's an example using that similar style where there's two idents
> on a long if statement. What do you think?
I'm used to having anything inside parentheses go just inside the
parentheses in subsequent lines, as shown just below the "if" example in
CodingStyle:
*idxp = ((l1_idx << PORT_ARRAY_L1_SHIFT)
| (l2_idx << PORT_ARRAY_L2_SHIFT)
| (l3_idx << PORT_ARRAY_L3_SHIFT));
I guess it's what I'm used to. Hard to argue rationally for style.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
