From: Sylvain Munaut tnt@246tNt.com

Currently msgb_pull returns a pointer to the new start of msgb, which is pretty counter intuitive and when using msgb_pull is often annoying.

For eg, the msgb_pull_u{8,16,32} were previously "fixed" for this quirk in a previous commit. The msgb_pull_l2h in lapdm is also wrong because of this, same for code in osmocom-bb loader.

AFAICT, the current users of msgb_pull either don't care about the return value, or expect it to be a pointer to the pulled header and not the new start of msgb (and so are currently buggy).

Without this patch, you have things like:

struct foo *bar = msgb_pull(msg, sizeof(struct foo)) - sizeof(struct foo)

With the patch, you have:

struct foo *bar = msgb_pull(msg, sizeof(struct foo));

which is IMHO a much better style.

Signed-off-by: Sylvain Munaut tnt@246tNt.com --- include/osmocom/core/msgb.h | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/include/osmocom/core/msgb.h b/include/osmocom/core/msgb.h index a1939ab..4a72f2d 100644 --- a/include/osmocom/core/msgb.h +++ b/include/osmocom/core/msgb.h @@ -287,16 +287,18 @@ static inline unsigned char *msgb_push(struct msgb *msgb, unsigned int len) /*! \brief remove (pull) a header from the front of the message buffer * \param[in] msgb message buffer * \param[in] len number of octets to be pulled - * \returns pointer to new start of msgb + * \returns pointer to header that's been pulled (i.e. previous start of msgb) * * This function moves the \a data pointer of the \ref msgb further back * in the message, thereby shrinking the size of the message by \a len * bytes. */ static inline unsigned char *msgb_pull(struct msgb *msgb, unsigned int len) -{ +{ + unsigned char *tmp = msgb->data; msgb->len -= len; - return msgb->data += len; + msgb->data += len; + return tmp; }

/*! \brief remove uint8 from front of message @@ -305,7 +307,7 @@ static inline unsigned char *msgb_pull(struct msgb *msgb, unsigned int len) */ static inline uint8_t msgb_pull_u8(struct msgb *msgb) { - uint8_t *space = msgb_pull(msgb, 1) - 1; + uint8_t *space = msgb_pull(msgb, 1); return space[0]; } /*! \brief remove uint16 from front of message @@ -314,7 +316,7 @@ static inline uint8_t msgb_pull_u8(struct msgb *msgb) */ static inline uint16_t msgb_pull_u16(struct msgb *msgb) { - uint8_t *space = msgb_pull(msgb, 2) - 2; + uint8_t *space = msgb_pull(msgb, 2); return space[0] << 8 | space[1]; } /*! \brief remove uint32 from front of message @@ -323,7 +325,7 @@ static inline uint16_t msgb_pull_u16(struct msgb *msgb) */ static inline uint32_t msgb_pull_u32(struct msgb *msgb) { - uint8_t *space = msgb_pull(msgb, 4) - 4; + uint8_t *space = msgb_pull(msgb, 4); return space[0] << 24 | space[1] << 16 | space[2] << 8 | space[3]; }