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
