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