Attention is currently required from: pespin.

Patch set 2:Code-Review -1

View Change

8 comments:

• File src/osmo-bts-trx/trx_if.c:

  • Patch Set #2, Line 1298: trx_udp_open

    I still think having a helper function is better to avoid code duplication and copy-paste errors.

  • Patch Set #2, Line 147: msg->data_len

    `msgb_length(msg)` here and below

  • Patch Set #2, Line 1006:

    		LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
    		
    

    ```suggestion
    		LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
    			"recv() failed on TRXD with rc=%d (%s)\n", res, errbuf);
    ```

  • Patch Set #2, Line 1110: trx_if_send_burst

    BTW, we may further improve this API to avoid `static` variables.

    My idea is to have the following API (draft):

    ```
    struct msgb *trx_if_data_msgb_alloc(void);
    struct msgb *trx_if_data_msgb_push_br(struct msgb *msg,
                                          const struct trx_dl_burst_req *br);
    struct msgb *trx_if_data_msgb_send(struct trx_l1h *l1h, struct msgb *msg);
    ```

  • Patch Set #2, Line 1128: buf = msgb_data(trx_data_last_msg);

    setting `buf` here is probably not needed?

  • Patch Set #2, Line 1164:

    ```suggestion
    			msgb_put_u32(trx_data_last_msg, br->fn);
    ```

  • Patch Set #2, Line 1195:

    		LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
    			"send() failed on TRXD with rc=%d (%s)\n",
    		
    

    We're no longer calling `send()` here, right?

    ```suggestion
    		LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
    			"osmo_iofd_write_msgb() failed on TRXD with rc=%d (%s)\n",
    			rc, errbuf);
    ```

  • Patch Set #2, Line 1319: i

    Copy-paste!

    ```suggestion
    	if (!l1h->trx_data_iofd) {
    ```

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