4 Mar
2014
4 Mar
'14
2:44 p.m.
Writing directly to following struct fields may cause inconsistencies
that are hard to debug:
data_len, len, head, tail, data
In general, the available macros and functions should be used to
modify them instead.
This patch declares these fields as const if
MSGB_DISABLE_DIRECT_WRITE is defined. Doing so may lead to warnings
and errors, therefore this macro is only defined for libosmocore yet,
where at least the errors are also fixed by this patch.
The main purpose is to maintain consistency, so only modifing the
fields themselves is restricted. It's still possible to modify the
data the pointers refer to.
Sponsored-by: On-Waves ehf
---
Doxyfile.core.in | 2 +-
Makefile.am | 3 +-
include/osmocom/core/msgb.h | 66 +++++++++++++++++++++++++++++-------------
src/msgb.c | 20 ++++++-------
tests/gsm0808/gsm0808_test.c | 3 +-
5 files changed, 60 insertions(+), 34 deletions(-)
diff --git a/Doxyfile.core.in b/Doxyfile.core.in
index 71843cc..eecd123 100644
--- a/Doxyfile.core.in
+++ b/Doxyfile.core.in
@@ -1447,7 +1447,7 @@ INCLUDE_FILE_PATTERNS =
# undefined via #undef or recursively expanded use the := operator
# instead of the = operator.
-PREDEFINED =
+PREDEFINED = DOXYGEN
# If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then
# this tag can be used to specify a list of macro names that should be expanded.
diff --git a/Makefile.am b/Makefile.am
index a77bd8f..d6a1992 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,6 +1,7 @@
ACLOCAL_AMFLAGS = -I m4
-AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include
+export AM_CPPFLAGS = -DMSGB_DISABLE_DIRECT_WRITE
+
SUBDIRS = include src src/vty src/codec src/gsm src/gb tests utils
pkgconfigdir = $(libdir)/pkgconfig
diff --git a/include/osmocom/core/msgb.h b/include/osmocom/core/msgb.h
index 33e8081..3593309 100644
--- a/include/osmocom/core/msgb.h
+++ b/include/osmocom/core/msgb.h
@@ -38,6 +38,12 @@
#define MSGB_DEBUG
+#ifdef MSGB_DISABLE_DIRECT_WRITE
+# define MSGB_CONST const
+#else
+# define MSGB_CONST
+#endif
+
/*! \brief Osmocom message buffer */
struct msgb {
struct llist_head list; /*!< \brief linked list header */
@@ -58,12 +64,32 @@ struct msgb {
unsigned long cb[5]; /*!< \brief control buffer */
- uint16_t data_len; /*!< \brief length of underlying data array */
- uint16_t len; /*!< \brief length of bytes used in msgb */
+ union {
+ /*! \brief length of underlying data array */
+ MSGB_CONST uint16_t data_len;
+ uint16_t __data_len;
+ };
+ union {
+ /*! \brief length of bytes used in msgb */
+ MSGB_CONST uint16_t len;
+ uint16_t __len;
+ };
+ union {
+ /*! \brief start of underlying memory buffer */
+ unsigned char * MSGB_CONST head;
+ unsigned char *__head;
+ };
+ union {
+ /*! \brief end of message in buffer */
+ unsigned char * MSGB_CONST tail;
+ unsigned char *__tail;
+ };
+ union {
+ /*! \brief start of message in buffer */
+ unsigned char * MSGB_CONST data;
+ unsigned char *__data;
+ };
- unsigned char *head; /*!< \brief start of underlying memory buffer */
- unsigned char *tail; /*!< \brief end of message in buffer */
- unsigned char *data; /*!< \brief start of message in buffer */
unsigned char _data[0]; /*!< \brief optional immediate data array */
};
@@ -179,12 +205,12 @@ static inline int msgb_headroom(const struct msgb *msgb)
*/
static inline unsigned char *msgb_put(struct msgb *msgb, unsigned int len)
{
- unsigned char *tmp = msgb->tail;
+ unsigned char *tmp = msgb->__tail;
if (msgb_tailroom(msgb) < (int) len)
MSGB_ABORT(msgb, "Not enough tailroom msgb_push (%u < %u)\n",
msgb_tailroom(msgb), len);
- msgb->tail += len;
- msgb->len += len;
+ msgb->__tail += len;
+ msgb->__len += len;
return tmp;
}
@@ -228,12 +254,12 @@ static inline void msgb_put_u32(struct msgb *msgb, uint32_t word)
*/
static inline unsigned char *msgb_get(struct msgb *msgb, unsigned int len)
{
- unsigned char *tmp = msgb->data - len;
+ unsigned char *tmp = msgb->__data - len;
if (msgb_length(msgb) < len)
MSGB_ABORT(msgb, "msgb too small to get %u (len %u)\n",
len, msgb_length(msgb));
- msgb->tail -= len;
- msgb->len -= len;
+ msgb->__tail -= len;
+ msgb->__len -= len;
return tmp;
}
/*! \brief remove uint8 from end of message
@@ -281,9 +307,9 @@ static inline unsigned char *msgb_push(struct msgb *msgb, unsigned int
len)
if (msgb_headroom(msgb) < (int) len)
MSGB_ABORT(msgb, "Not enough headroom msgb_push (%u < %u)\n",
msgb_headroom(msgb), len);
- msgb->data -= len;
- msgb->len += len;
- return msgb->data;
+ msgb->__data -= len;
+ msgb->__len += len;
+ return msgb->__data;
}
/*! \brief remove (pull) a header from the front of the message buffer
* \param[in] msgb message buffer
@@ -296,8 +322,8 @@ static inline unsigned char *msgb_push(struct msgb *msgb, unsigned int
len)
*/
static inline unsigned char *msgb_pull(struct msgb *msgb, unsigned int len)
{
- msgb->len -= len;
- return msgb->data += len;
+ msgb->__len -= len;
+ return msgb->__data += len;
}
/*! \brief remove (pull) all headers in front of l3h from the message buffer.
@@ -356,8 +382,8 @@ static inline uint32_t msgb_pull_u32(struct msgb *msgb)
*/
static inline void msgb_reserve(struct msgb *msg, int len)
{
- msg->data += len;
- msg->tail += len;
+ msg->__data += len;
+ msg->__tail += len;
}
/*! \brief Trim the msgb to a given absolute length
@@ -370,8 +396,8 @@ static inline int msgb_trim(struct msgb *msg, int len)
if (len > msg->data_len)
return -1;
- msg->len = len;
- msg->tail = msg->data + len;
+ msg->__len = len;
+ msg->__tail = msg->__data + len;
return 0;
}
diff --git a/src/msgb.c b/src/msgb.c
index b2fe1d2..1106824 100644
--- a/src/msgb.c
+++ b/src/msgb.c
@@ -55,11 +55,11 @@ struct msgb *msgb_alloc(uint16_t size, const char *name)
return NULL;
}
- msg->data_len = size;
- msg->len = 0;
- msg->data = msg->_data;
- msg->head = msg->_data;
- msg->tail = msg->_data;
+ msg->__data_len = size;
+ msg->__len = 0;
+ msg->__data = msg->_data;
+ msg->__head = msg->_data;
+ msg->__tail = msg->_data;
return msg;
}
@@ -113,10 +113,10 @@ struct msgb *msgb_dequeue(struct llist_head *queue)
*/
void msgb_reset(struct msgb *msg)
{
- msg->len = 0;
- msg->data = msg->_data;
- msg->head = msg->_data;
- msg->tail = msg->_data;
+ msg->__len = 0;
+ msg->__data = msg->_data;
+ msg->__head = msg->_data;
+ msg->__tail = msg->_data;
msg->trx = NULL;
msg->lchan = NULL;
@@ -133,7 +133,7 @@ void msgb_reset(struct msgb *msg)
*/
uint8_t *msgb_data(const struct msgb *msg)
{
- return msg->data;
+ return msg->__data;
}
/*! \brief get length of message buffer
diff --git a/tests/gsm0808/gsm0808_test.c b/tests/gsm0808/gsm0808_test.c
index 7e5e97b..1ce6ef9 100644
--- a/tests/gsm0808/gsm0808_test.c
+++ b/tests/gsm0808/gsm0808_test.c
@@ -108,8 +108,7 @@ static void test_create_cipher_complete()
msgb_free(msg);
/* with l3 data but short */
- l3->len -= 1;
- l3->tail -= 1;
+ msgb_trim(l3, l3->len - 1);
msg = gsm0808_create_cipher_complete(l3, 4);
VERIFY(msg, res2, ARRAY_SIZE(res2));
msgb_free(msg);
--
1.7.9.5