Re: [PATCH 5/6] gsup/oap: add OAP to GSUP client.

[Neels Hofmeyr]

On Mon, Nov 02, 2015 at 03:06:29PM +0100, Neels Hofmeyr wrote:
> On Mon, Nov 02, 2015 at 11:33:50AM +0100, Holger Freyther wrote:
> > > static int gsup_client_read_cb(struct ipa_client_conn *link, struct msgb 
> > > *msg)
> > > {
> > > 
> > > - OSMO_ASSERT(gsupc->read_cb != NULL);
> > > - gsupc->read_cb(gsupc, msg);
> > > + if (he->proto == IPAC_PROTO_EXT_GSUP) {
> > > +         OSMO_ASSERT(gsupc->read_cb != NULL);
> > > +         gsupc->read_cb(gsupc, msg);
> > > +         /* expecting read_cb() to free msg */
> > > + }
> > > + else
> > > + if (he->proto == IPAC_PROTO_EXT_OAP) {
> > > +         return gsup_client_oap_handle(gsupc, msg);
> > > +         /* gsup_client_oap_handle frees msg */
> > > + }
> > > + else
> > > +         goto invalid;
> > 
> > the coding style would not have else and if on two different lines. I will 
> > fix this myself
> > right now.
> 
> Yes, indeed. I'm doing that on purpose... the logical idea is that the if
> conditions all start on the same column.

Now that I'm actually looking at the commit, let's name it:

I wrote:

   if (a) {
           frizziply();
   }
   else 
   if (b) {
           frobnicate();
   }
   else
           goto invalid;

and you committed

   if (a) {
           frizziply();
   } else if (b) {
           frobnicate();
   } else
           goto invalid;

I personally find it mildly ugly, but let's avoid the discussion.

To reiterate, I'd like to know whether gtphub should/really must ;) be
changed to the latter style.

~Neels

signature.asc
Description: Digital signature