Change in osmo-bts[master]: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Harald Welte has submitted this change and it was merged. (
https://gerrit.osmocom.org/13768 )
Change subject: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
..
common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Found using clang-8:
rsl.c:1646:7: warning: taking address of packed member 'packets_sent'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1646:28: warning: taking address of packed member 'octets_sent'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1647:7: warning: taking address of packed member 'packets_recv'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1647:28: warning: taking address of packed member 'octets_recv'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1648:7: warning: taking address of packed member 'packets_lost'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1648:28: warning: taking address of packed member 'arrival_jitter'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40
---
M src/common/rsl.c
1 file changed, 23 insertions(+), 23 deletions(-)
Approvals:
Jenkins Builder: Verified
Harald Welte: Looks good to me, approved
diff --git a/src/common/rsl.c b/src/common/rsl.c
index 8f5d689..c2a7db6 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -1627,33 +1627,33 @@
*/
static void rsl_add_rtp_stats(struct gsm_lchan *lchan, struct msgb *msg)
{
- struct ipa_stats {
- uint32_t packets_sent;
- uint32_t octets_sent;
- uint32_t packets_recv;
- uint32_t octets_recv;
- uint32_t packets_lost;
- uint32_t arrival_jitter;
- uint32_t avg_tx_delay;
- } __attribute__((packed));
+ uint32_t packets_sent, octets_sent;
+ uint32_t packets_recv, octets_recv;
+ uint32_t packets_lost;
+ uint32_t arrival_jitter;
- struct ipa_stats stats;
+ msgb_tv_put(msg, RSL_IE_IPAC_CONN_STAT, sizeof(uint32_t) * 7);
- memset(, 0, sizeof(stats));
-
- if (lchan->abis_ip.rtp_socket)
+ if (lchan->abis_ip.rtp_socket) {
osmo_rtp_socket_stats(lchan->abis_ip.rtp_socket,
- _sent, _sent,
- _recv, _recv,
- _lost,
_jitter);
- /* convert to network byte order */
- stats.packets_sent = htonl(stats.packets_sent);
- stats.octets_sent = htonl(stats.octets_sent);
- stats.packets_recv = htonl(stats.packets_recv);
- stats.octets_recv = htonl(stats.octets_recv);
- stats.packets_lost = htonl(stats.packets_lost);
+ _sent, _sent,
+ _recv, _recv,
+ _lost, _jitter);
- msgb_tlv_put(msg, RSL_IE_IPAC_CONN_STAT, sizeof(stats), (uint8_t *)
);
+ /* msgb_put_u32() uses osmo_store32be(),
+* so we don't need to call htonl(). */
+ msgb_put_u32(msg, packets_sent);
+ msgb_put_u32(msg, octets_sent);
+ msgb_put_u32(msg, packets_recv);
+ msgb_put_u32(msg, octets_recv);
+ msgb_put_u32(msg, packets_lost);
+ msgb_put_u32(msg, arrival_jitter);
+ /* FIXME: AVG Tx delay is always 0 */
+ msgb_put_u32(msg, 0);
+ } else {
+ msgb_put(msg, sizeof(uint32_t) * 7);
+ memset(msg->tail, 0x00, sizeof(uint32_t) * 7);
+ }
}
int rsl_tx_ipac_dlcx_ind(struct gsm_lchan *lchan, uint8_t cause)
--
To view, visit https://gerrit.osmocom.org/13768
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40
Gerrit-Change-Number: 13768
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy
Gerrit-Reviewer: Harald Welte
Gerrit-Reviewer: Jenkins Builder (102)
Gerrit-Reviewer: Vadim Yanitskiy
Change in osmo-bts[master]: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13768 ) Change subject: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats() .. Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/13768 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Vadim Yanitskiy Gerrit-Comment-Date: Wed, 24 Apr 2019 08:47:01 + Gerrit-HasComments: No Gerrit-HasLabels: Yes
Change in osmo-bts[master]: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Vadim Yanitskiy has posted comments on this change. ( https://gerrit.osmocom.org/13768 ) Change subject: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats() .. Patch Set 1: > Maybe it's the __attribute((packed)) which is putting clang off? Yep, exactly. All warnings are triggered by policy -Waddress-of-packed-member. In general, I don't see big advantages of using a struct here, no matter is it packed or not. We still have to memset() it, and still need to convert all its members to big-endian... -- To view, visit https://gerrit.osmocom.org/13768 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Jenkins Builder (102) Gerrit-Reviewer: Vadim Yanitskiy Gerrit-CC: Harald Welte Gerrit-Comment-Date: Wed, 24 Apr 2019 08:07:18 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-bts[master]: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13768 ) Change subject: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats() .. Patch Set 1: I actually think this is bogus. We are not type-casting uint32_t values (or structs containing uint32_t) to random locations in memory. Rather, we are puting a struct if uint32_t on the stack and then passing pointers to members of it. I would have assumed that the compiler puts the structure on the stack in a way that the uint32_t are properly aligned. After all, if you had all those uint32_t individually on the stack, you would also expect they're all 32bit aligned and you could use them as uint32_t. Maybe it's the __attribute((packed)) which is putting clang off? It's not really needed here, as all (I hope at least) architectures are putting two uint32_t next to each other in a struct, without any padding. Maybe there were any early "true" 64bit architectures like ppc64 or itanium or sparc65 that would align them on 64bit boundaries, but I somehow doubt it. -- To view, visit https://gerrit.osmocom.org/13768 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40 Gerrit-Change-Number: 13768 Gerrit-PatchSet: 1 Gerrit-Owner: Vadim Yanitskiy Gerrit-Reviewer: Jenkins Builder (102) Gerrit-CC: Harald Welte Gerrit-Comment-Date: Wed, 24 Apr 2019 07:47:17 + Gerrit-HasComments: No Gerrit-HasLabels: No
Change in osmo-bts[master]: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Vadim Yanitskiy has uploaded this change for review. (
https://gerrit.osmocom.org/13768
Change subject: common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
..
common/rsl.c: fix unaligned pointers in rsl_add_rtp_stats()
Found using clang-8:
rsl.c:1646:7: warning: taking address of packed member 'packets_sent'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1646:28: warning: taking address of packed member 'octets_sent'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1647:7: warning: taking address of packed member 'packets_recv'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1647:28: warning: taking address of packed member 'octets_recv'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1648:7: warning: taking address of packed member 'packets_lost'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
rsl.c:1648:28: warning: taking address of packed member 'arrival_jitter'
of class or structure 'ipa_stats' may result in an
unaligned pointer value
Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40
---
M src/common/rsl.c
1 file changed, 23 insertions(+), 23 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/68/13768/1
diff --git a/src/common/rsl.c b/src/common/rsl.c
index 8f5d689..c2a7db6 100644
--- a/src/common/rsl.c
+++ b/src/common/rsl.c
@@ -1627,33 +1627,33 @@
*/
static void rsl_add_rtp_stats(struct gsm_lchan *lchan, struct msgb *msg)
{
- struct ipa_stats {
- uint32_t packets_sent;
- uint32_t octets_sent;
- uint32_t packets_recv;
- uint32_t octets_recv;
- uint32_t packets_lost;
- uint32_t arrival_jitter;
- uint32_t avg_tx_delay;
- } __attribute__((packed));
+ uint32_t packets_sent, octets_sent;
+ uint32_t packets_recv, octets_recv;
+ uint32_t packets_lost;
+ uint32_t arrival_jitter;
- struct ipa_stats stats;
+ msgb_tv_put(msg, RSL_IE_IPAC_CONN_STAT, sizeof(uint32_t) * 7);
- memset(, 0, sizeof(stats));
-
- if (lchan->abis_ip.rtp_socket)
+ if (lchan->abis_ip.rtp_socket) {
osmo_rtp_socket_stats(lchan->abis_ip.rtp_socket,
- _sent, _sent,
- _recv, _recv,
- _lost,
_jitter);
- /* convert to network byte order */
- stats.packets_sent = htonl(stats.packets_sent);
- stats.octets_sent = htonl(stats.octets_sent);
- stats.packets_recv = htonl(stats.packets_recv);
- stats.octets_recv = htonl(stats.octets_recv);
- stats.packets_lost = htonl(stats.packets_lost);
+ _sent, _sent,
+ _recv, _recv,
+ _lost, _jitter);
- msgb_tlv_put(msg, RSL_IE_IPAC_CONN_STAT, sizeof(stats), (uint8_t *)
);
+ /* msgb_put_u32() uses osmo_store32be(),
+* so we don't need to call htonl(). */
+ msgb_put_u32(msg, packets_sent);
+ msgb_put_u32(msg, octets_sent);
+ msgb_put_u32(msg, packets_recv);
+ msgb_put_u32(msg, octets_recv);
+ msgb_put_u32(msg, packets_lost);
+ msgb_put_u32(msg, arrival_jitter);
+ /* FIXME: AVG Tx delay is always 0 */
+ msgb_put_u32(msg, 0);
+ } else {
+ msgb_put(msg, sizeof(uint32_t) * 7);
+ memset(msg->tail, 0x00, sizeof(uint32_t) * 7);
+ }
}
int rsl_tx_ipac_dlcx_ind(struct gsm_lchan *lchan, uint8_t cause)
--
To view, visit https://gerrit.osmocom.org/13768
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifba33cfd8edeccc99a21c7d076db7119c29d4f40
Gerrit-Change-Number: 13768
Gerrit-PatchSet: 1
Gerrit-Owner: Vadim Yanitskiy
