Attention is currently required from: fixeria, lynxis lazus, pespin.

View Change

2 comments:

• File include/osmocom/gprs/gmm/gmm_prim.h:

  • Patch Set #1, Line 369: /* Alloc primitive for GMMBSSGP SAP: */

    I really prefer getting this merged as is since it gives a clear view on the primitives being expect […]

    objectively, the names are unusually long indeed.

    It sounds like fixeria suggests a more maintainable public API.

    I'm not entirely clear though, fixeria, on what exactly you mean by "let the user fill prim specific fields using designated initializers (or whatever they like)"

    are we suggesting something like this?

    ```
    my_prim = osmo_gmm_prim_gmmbssgp_alloc();
    osmo_gmm_prim_set_bvci(my_prim, my_bvci);
    osmo_gmm_prim_set_nsei(my_prim, my_nsei);
    ```

    or like this?
    ```
    struct osmo_gprs_gmm_prim_rcu_args {
      uint16_t bvci;
      uint16_t nsei;
    };
    my_rcu_args = (struct osmo_gprs_gmm_prim_rcu_args){...};
    my_prim = osmo_gsmm_prim_alloc_rcu(&my_rcu_args);
    ```

    Every way does have its own drawbacks and also we don't seem to have a clear culture of one way or the other in osmocom... so i'd let the patch submitter decide on this stuff, as long as it works.

    For certain, objectively, these names are very very long. It's up in the same category with the osmo-msc logging of context that I wrote back in the days, almost breaching on the unmanageable.

• File src/gmm/gmm_prim.c:

  • Patch Set #1, Line 353: gmm_prim->gmmbssgp.nsei = nsei;

    in here it does look like a lot of repetition that could be more elegant with API like the pseudocode in my other comment -- pass in the OSMO_GPRS_GMM_ constants as arguments instead of adding a lot of binary objects for each constant.

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