Attention is currently required from: fixeria, dexter.

View Change

5 comments:

• File src/osmobts_sock.c:

  • Patch Set #5, Line 115: bts->trx[trx].fl1h = NULL;

    After another looks, this looks even worse, because you are not skipping setting fl1h to NULL if phy_ops is not defined. Are you shure that's fine?
    I'd really do a 2 level if() first checking for fl1h and later calling the function ig phy_ops.l1if_close_pdch exists.

• File src/pcu_l1_if.cpp:

  • Patch Set #5, Line 218: the_pcu->phy_ops->l1if_pdch_req(bts->trx[trx].fl1h, ts, 0, fn, arfcn, block_nr,

    guard this with "if (the_pcu->phy_ops->l1if_pdch_req)". Similar for all other function calls.

• File src/pcu_l1_if_phy.c:

  • Patch Set #5, Line 15: struct llist_head list;

    You can really put this inside ops too, and get rid of l1if_phy_entry.

  • Patch Set #5, Line 18: const char *name;

    this can really be in li1f_phy_ops and assign the pointer statically, there's no need to have it separate. IT makes more sense to have it inside the ops since it goes together, it identifies a given set of ops.

  • Patch Set #5, Line 27: bool singleton;

    this too can be set in phy_ops. I don't see the need of passing several params to register(), one is enough.

To view, visit change 31341. To unsubscribe, or for help writing mail filters, visit settings.