[PATCH] core/msgb: Change return value of msgb_pull to the start of pulled header

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/baseband-devel@lists.osmocom.org/.

Sylvain Munaut 246tnt at gmail.com
Sat Jan 5 14:52:03 UTC 2013


From: Sylvain Munaut <tnt at 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 at 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];
 }
 
-- 
1.7.8.6





More information about the baseband-devel mailing list