I just came across this compiler warning:
l1sap.c: In function 'gsmtap_ph_rach': l1sap.c:291:8: warning: assignment from incompatible pointer type [enabled by default] *data = &l1sap->u.rach_ind.ra; ^
It's a uint16_t ra and used like this in common/l1sap.c:
static int gsmtap_ph_rach(struct osmo_phsap_prim *l1sap, uint8_t *chan_type, uint8_t *tn, uint8_t *ss, uint32_t *fn, uint8_t **data, int *len) { [...] *data = &l1sap->u.rach_ind.ra; *len = 1;
A cast like this would fix the compiler warning and make sense if data is interpreted to point at a 16 bit data block, probably in host byte order though:
*data = (uint8_t*)&l1sap->u.rach_ind.ra; *len = 1;
But *len = 1 means that **data is an 8bit value? This is sent to gsmtap.
Is *len = 1 something we should fix?
~Neels
That's probably related to https://gerrit.osmocom.org/#/c/427/3 If that's really the case than fix would be to check for is_11bit of ph_rach_ind_param and adjust length accordingly.
On 07/25/2016 12:22 PM, Neels Hofmeyr wrote:
I just came across this compiler warning:
l1sap.c: In function 'gsmtap_ph_rach': l1sap.c:291:8: warning: assignment from incompatible pointer type [enabled by default] *data = &l1sap->u.rach_ind.ra; ^
It's a uint16_t ra and used like this in common/l1sap.c:
static int gsmtap_ph_rach(struct osmo_phsap_prim *l1sap, uint8_t *chan_type, uint8_t *tn, uint8_t *ss, uint32_t *fn, uint8_t **data, int *len) { [...] *data = &l1sap->u.rach_ind.ra; *len = 1;
A cast like this would fix the compiler warning and make sense if data is interpreted to point at a 16 bit data block, probably in host byte order though:
*data = (uint8_t*)&l1sap->u.rach_ind.ra; *len = 1;But *len = 1 means that **data is an 8bit value? This is sent to gsmtap.
Is *len = 1 something we should fix?
~Neels
On 25 Jul 2016, at 18:31, Max msuraev@sysmocom.de wrote:
That's probably related to https://gerrit.osmocom.org/#/c/427/3 If that's really the case than fix would be to check for is_11bit of ph_rach_ind_param and adjust length accordingly.
right, https://gerrit.osmocom.org/#/c/434/2/src/osmo-bts-sysmo/l1_if.c has the potential fix/update to it. I re-triggered the jenkins job right now. The BTS/PCU protocol is next, followed by the PCU. :)
holger
On 25 Jul 2016, at 18:22, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
I just came across this compiler warning:
l1sap.c: In function 'gsmtap_ph_rach': l1sap.c:291:8: warning: assignment from incompatible pointer type [enabled by default] *data = &l1sap->u.rach_ind.ra; ^
okay, I reviewed the patch that adds 11bit support for the sysmobts but l1sap.c is not addressed. Shall we revert the 11bit l1sap change for now or wait?
holger
PS: If we would disable the doxygen warnings and remove -Wall when compiling the dependencies the amount of warnings would go down and we might find such things earlier?
On Mon, Jul 25, 2016 at 08:40:37PM +0800, Holger Freyther wrote:
On 25 Jul 2016, at 18:22, Neels Hofmeyr nhofmeyr@sysmocom.de wrote:
I just came across this compiler warning:
l1sap.c: In function 'gsmtap_ph_rach': l1sap.c:291:8: warning: assignment from incompatible pointer type [enabled by default] *data = &l1sap->u.rach_ind.ra; ^
okay, I reviewed the patch that adds 11bit support for the sysmobts but l1sap.c is not addressed. Shall we revert the 11bit l1sap change for now or wait?
This "only" concerns gsmtap. Given the lengthy course of those patches, I'd give it some time and not add frustration with a revert... Bhagarva, are you reading this, and could you add gsmtap capability for 11 bit soon?
PS: If we would disable the doxygen warnings and remove -Wall when compiling the dependencies the amount of warnings would go down and we might find such things earlier?
+1 for disabling doxygen warnings (or >/dev/null 2>&1 even)
Otherwise, the way I notice warnings is mostly by running ':make' in vim and browsing the warnings. We don't have that many warnings with -Wall, do we? Most of them are usually '#warning' or deprecation warnings. I'd keep -Wall.
~Neels
-
Holger Freyther -
Max -
Neels Hofmeyr