Attention is currently required from: arehbein, fixeria, daniel.

View Change

5 comments:

• File src/gsm/ipa.c:

  • Patch Set #8, Line 728: * -EIO, if the header declares a payload too large */

    */ on the next line

  • Patch Set #8, Line 729: int ipa_segmentation_cb(struct msgb *msg)

    as per struct osmo_io_ops documentation:
    			/*! call-back function to segment the data at message boundaries.
    			 *  Needs to return the size of the next message. If it returns
    			 *  -EAGAIN or a value larger than msgb_length() (message is incomplete)
    			 *  osmo_io will wait for more data to be read. Other negative values
    			 *  cause the msg to be discarded. */
    			int (*segmentation_cb)(struct msgb *msg);

  • Patch Set #8, Line 735: const struct ipaccess_head *hh = (const struct ipaccess_head *) msg->data;

    You could declare this at the start of the function (we usually declare variables at the start of the block).
    Then, you could already use sizeof(*hh) in line 731, instead of using different things for sizeof() as in line 737 which makes it confusing to read.

  • Patch Set #8, Line 737: size_t total_len = payload_len + sizeof(*hh);

    "sizeof(*hh) + payload_len;" it's logically easier to understand, as in lefto-to-right order filling.

  • Patch Set #8, Line 738: if (msgb_tailroom(msg) + msgb_length(msg) < total_len) {

    iiuc the problem here is that the allocated msgb space is not going to be enough to fit in what IPA message expects.
    Talking about that in the log message would be a lot more usefu for the user? since that probably means the dev has to increase the msgb size when allocating it?

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