Attention is currently required from: fixeria, pespin, daniel.
8 comments:
File src/gsm/ipa.c:
Patch Set #8, Line 728: * -EIO, if the header declares a payload too large */
*/ on the next line
hmm weird, I remember reading somewhere that Osmocom comments do *not* put the comment end marker on the same line. Admittedly, I don't know where I read it, but I'm seeing the same style in lots of other places.
Patch Set #8, Line 729: int ipa_segmentation_cb(struct msgb *msg)
as per struct osmo_io_ops documentation: […]
So what's wrong with the Doxygen style comment? It doesn't contradict the description you quote.
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 th […]
Done
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 […]
Done
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 IP […]
Done
File src/gsm/ipa.c:
Patch Set #10, Line 733: osmo_ntohs(hh->len);
Now the problem is that you're accessing the buffer before checking if there is enough data in it.
ah yeah thanks, had that in mind, but forgot it again in between all the other CR comments/changes and rebasing.
Patch Set #10, Line 735: msgb_length(msg) + msgb_tailroom(msg);
This is incorrect. […]
I think you got confused here about `msg->data_len`, `msg->len` and `msgb_length()`.
`msg->data_len` includes all the bytes allocated (including tailroom and headroom); `msg->len` includes all the bytes reserved for the message to be stored (i.e. not the tailroom and not the headroom).
At least that's what I'm getting from the code.
`ENOMEM` or `ENOSPC` is a better fit here, IMO.
I took a look at `errno -l` and found `ENOBUFS 105 No buffer space available`.
To view, visit change 33083. To unsubscribe, or for help writing mail filters, visit settings.