[PATCH 5/8] mgcp: NUL-terminate MGCP message

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Jacob Erlbeck jerlbeck at sysmocom.de
Mon Dec 2 10:38:27 UTC 2013


On 12/02/2013 11:14 AM, Holger Hans Peter Freyther wrote:
> On Mon, Dec 02, 2013 at 10:21:58AM +0100, Jacob Erlbeck wrote:
> 
>>>> +	/* Ensure that the msg->l2h is NULL terminated. */
>>>> +	if (msgb_tailroom(msg) > 0)
>>>> +		*msg->tail = '\0';
>>>> +	else if (*(msg->tail-1) == '\r' || *(msg->tail-1) == '\n')
>>>> +		*(msg->tail - 1) = '\0';
>>>> +	else {
>>>> +		return NULL;
>>>> +	}
>>>
>>> The check misses if "tail - 1" is already \0
>> Yes, but does this hurt? I wouldn't expect l2 to end with a \0 since the
>> protocol is defined without one (see below). And even if there was one,
>> a second one wouldn't be a problem.
> 
> tailroom == 0 AND  msg->l2h[msgb_length(msg) - 1] == '\0' is rejected
> by your above code.

Ok, agreed. And to be complete, the test of the length of the data arae
being > 0 is missing, too.

So I'll add sth like:

else if (msgb_l2len(msg) > 0 && *(msg->tail-1) == '\0')
      /* do nothing */
else ... '\r' ...

Or do you rather recommend to use msg->l2h + msgb_l2len(msg) instead of
msg->tail directly?

And why do you use msgb_length() and not msgb_l2len()?

Jacob





More information about the OpenBSC mailing list