2 Mar
2014
2 Mar
'14
8:24 a.m.
On Fri, Feb 28, 2014 at 08:47:46PM +0100, Jacob Erlbeck wrote:
Good Morning,
I welcome any change that makes it more difficult to create the
mess that LAPD/LAPDm is in. I have some questions and comments
though.
Writing directly to following struct fields may cause
inconsistencies
that hard to debug:
In general, the available macros and functions should
be used to
modify them instead.
^ must as in RFC?
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.
I will get back to this later as well. Have you looked at GSEAL
of GTK+ 2.0? Their goal was to hide direct access to variables too
and they introduced a macro like:
#ifndef GSEAL
/* introduce GSEAL() here for all of Gdk and Gtk+ without the need to modify GLib */
# ifdef GSEAL_ENABLE
# define GSEAL(ident) _g_sealed__ ## ident
# else
# define GSEAL(ident) ident
# endif
#endif /* !GSEAL */
It is not making the variables const but hiding them with a rename
and causing a compilation failure when being accessed.
-AM_CPPFLAGS = $(all_includes)
-I$(top_srcdir)/include
+export AM_CPPFLAGS = $(all_includes) -I$(top_srcdir)/include -DMSGB_DISABLE_DIRECT_WRITE
Is that really necessary?
+#ifdef DOXYGEN
My take would be that we try to avoid direct access so we don't need
to document these variables and can use \internal here. I think it is
preferable to avoid the situation where the ABI changes based on an
external define (even if it is just for documentation now). I have seen
a lot of wild CFLAGS/CPPFLAGS that users set while working on OpenEmbedded
+ const uint16_t data_len; /*!< \brief length of
underlying data array */
+ const uint16_t len; /*!< \brief length of bytes used in msgb */
+ unsigned char const *head; /*!< \brief start of underlying memory buffer */
+ unsigned char const *tail; /*!< \brief end of message in buffer */
+ unsigned char const *data; /*!< \brief start of message in buffer */
+#else
+ union {
+ MSGB_CONST uint16_t data_len;
+ uint16_t __data_len;
+ };
Anyone knows an ABI where alignment for const/non-const is different? :)
cheers
holger