tnt has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-e1d/+/26808 )
Change subject: Fix a bunch of warning raised by the new build warning options ......................................................................
Fix a bunch of warning raised by the new build warning options
A lot of them are related to signedness or type range limitation. A lot are not actual issues and work find in practice, but a few lead to actual bad behavior.
This makes all the conversion explicit to mark intent.
Signed-off-by: Sylvain Munaut tnt@246tNt.com Change-Id: I992b9bc51659e85966651b1146091501b8f149f5 --- M src/ctl.c M src/e1gen/osmo_e1f.c M src/mux_demux.c M src/proto_srv.c M src/usb.c M src/vty.c 6 files changed, 12 insertions(+), 15 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-e1d refs/changes/08/26808/1
diff --git a/src/ctl.c b/src/ctl.c index b633b9b..4a922b9 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -264,10 +264,7 @@ if (!line) return 0;
- n = (hdr->ts == E1DP_INVALID) ? 32 : ( - ((hdr->ts >= 0) && (hdr->ts < 31)) ? 1 : 0 - ); - + n = (hdr->ts == E1DP_INVALID) ? 32 : ((hdr->ts < 31) ? 1 : 0); if (!n) return 0;
diff --git a/src/e1gen/osmo_e1f.c b/src/e1gen/osmo_e1f.c index fa64e17..9d4f808 100644 --- a/src/e1gen/osmo_e1f.c +++ b/src/e1gen/osmo_e1f.c @@ -55,7 +55,7 @@ int osmo_e1f_instance_init(struct osmo_e1f_instance *e1i, const char *name, e1_notify_cb cb, bool crc4_enabled, void *priv) { - int i; + unsigned int i;
e1i->crc4_enabled = crc4_enabled; e1i->notify_cb = cb; @@ -101,7 +101,7 @@ * \param[in] e1t E1 instance which we are to stop */ void osmo_e1f_instance_reset(struct osmo_e1f_instance *e1i) { - int i; + unsigned int i;
align_fsm_reset(e1i);
@@ -290,7 +290,7 @@ * \returns 0 on success, negative on error */ int osmo_e1f_pull_tx_frame(struct osmo_e1f_instance *e1i, uint8_t *out_frame) { - int i; + unsigned int i;
if (e1i->tx.ais) { memset(out_frame, 0xff, 32); @@ -684,7 +684,7 @@ */ int osmo_e1f_rx_frame(struct osmo_e1f_instance *e1i, const uint8_t *in_frame) { - int i; + unsigned int i;
e1_rx_update_crc4(e1i, in_frame);
diff --git a/src/mux_demux.c b/src/mux_demux.c index a588829..efd7ac8 100644 --- a/src/mux_demux.c +++ b/src/mux_demux.c @@ -87,7 +87,7 @@ if (!ts->hdlc.tx_len) { rv = recv(ts->fd, ts->hdlc.tx_buf, sizeof(ts->hdlc.tx_buf), MSG_TRUNC); if (rv > 0) { - if (rv > sizeof(ts->hdlc.tx_buf)) { + if (rv > (int)sizeof(ts->hdlc.tx_buf)) { LOGPTS(ts, DXFR, LOGL_ERROR, "Truncated message: Client tried to " "send %d bytes but our buffer is limited to %lu\n", rv, sizeof(ts->hdlc.tx_buf)); @@ -151,7 +151,7 @@ LOGPTS(ts, DE1D, LOGL_ERROR, "dead socket during read: %s\n", strerror(errno)); e1_ts_stop(ts); - } else if (l < len) { + } else if (l < (int)len) { LOGPTS(ts, DE1D, LOGL_NOTICE, "TS read underflow: We had %zu bytes to read, " "but socket returned only %d\n", len, l); } @@ -297,7 +297,7 @@ LOGPTS(ts, DE1D, LOGL_ERROR, "dead socket during write: %s\n", strerror(errno)); e1_ts_stop(ts); - } else if (rv < len) { + } else if (rv < (int)len) { LOGPTS(ts, DE1D, LOGL_NOTICE, "TS write overflow: We had %zu bytes to send, " "but write returned only %d\n", len, rv); } diff --git a/src/proto_srv.c b/src/proto_srv.c index c376423..d6d3d17 100644 --- a/src/proto_srv.c +++ b/src/proto_srv.c @@ -94,7 +94,7 @@
/* Check payload length */ if ((h->payload_len >= 0) && - (h->payload_len != (msgb_length(msgb) - sizeof(struct osmo_e1dp_msg_hdr)))) + (h->payload_len != (int)(msgb_length(msgb) - sizeof(struct osmo_e1dp_msg_hdr)))) { LOGP(DE1D, LOGL_ERROR, "Invalid payload for message type: %d / (%d/%d/%d).\n", hdr->type, hdr->intf, hdr->line, hdr->ts); diff --git a/src/usb.c b/src/usb.c index d5107bc..32a249a 100644 --- a/src/usb.c +++ b/src/usb.c @@ -189,7 +189,7 @@ flow->cb(flow, libusb_get_iso_packet_buffer_simple(xfr, j), (xfr->iso_packet_desc[j].status == LIBUSB_TRANSFER_COMPLETED) ? - xfr->iso_packet_desc[j].actual_length : -1 + (int)xfr->iso_packet_desc[j].actual_length : -1 ); len += (xfr->iso_packet_desc[j].length = flow->size); } @@ -359,7 +359,7 @@
switch (irq->type) { case ICE1USB_IRQ_T_ERRCNT: - if (xfer->actual_length < sizeof(*irq)) { + if (xfer->actual_length < (int)sizeof(*irq)) { LOGPLI(line, DE1D, LOGL_ERROR, "Short ERRCNT interrupt: %u<%zu\n", xfer->actual_length, sizeof(*irq)); break; diff --git a/src/vty.c b/src/vty.c index 563950d..2f2425c 100644 --- a/src/vty.c +++ b/src/vty.c @@ -107,7 +107,7 @@
static void vty_dump_line(struct vty *vty, const struct e1_line *line) { - int tn; + unsigned int tn;
vty_out(vty, "Interface #%u, Line #%u, Mode %s%s%s:%s", line->intf->id, line->id, get_value_string(e1_line_mode_names, line->mode),
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/26808 )
Change subject: Fix a bunch of warning raised by the new build warning options ......................................................................
Patch Set 1: Code-Review+2
laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/26808 )
Change subject: Fix a bunch of warning raised by the new build warning options ......................................................................
Fix a bunch of warning raised by the new build warning options
A lot of them are related to signedness or type range limitation. A lot are not actual issues and work find in practice, but a few lead to actual bad behavior.
This makes all the conversion explicit to mark intent.
Signed-off-by: Sylvain Munaut tnt@246tNt.com Change-Id: I992b9bc51659e85966651b1146091501b8f149f5 --- M src/ctl.c M src/e1gen/osmo_e1f.c M src/mux_demux.c M src/proto_srv.c M src/usb.c M src/vty.c 6 files changed, 12 insertions(+), 15 deletions(-)
Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved
diff --git a/src/ctl.c b/src/ctl.c index b633b9b..4a922b9 100644 --- a/src/ctl.c +++ b/src/ctl.c @@ -264,10 +264,7 @@ if (!line) return 0;
- n = (hdr->ts == E1DP_INVALID) ? 32 : ( - ((hdr->ts >= 0) && (hdr->ts < 31)) ? 1 : 0 - ); - + n = (hdr->ts == E1DP_INVALID) ? 32 : ((hdr->ts < 31) ? 1 : 0); if (!n) return 0;
diff --git a/src/e1gen/osmo_e1f.c b/src/e1gen/osmo_e1f.c index fa64e17..9d4f808 100644 --- a/src/e1gen/osmo_e1f.c +++ b/src/e1gen/osmo_e1f.c @@ -55,7 +55,7 @@ int osmo_e1f_instance_init(struct osmo_e1f_instance *e1i, const char *name, e1_notify_cb cb, bool crc4_enabled, void *priv) { - int i; + unsigned int i;
e1i->crc4_enabled = crc4_enabled; e1i->notify_cb = cb; @@ -101,7 +101,7 @@ * \param[in] e1t E1 instance which we are to stop */ void osmo_e1f_instance_reset(struct osmo_e1f_instance *e1i) { - int i; + unsigned int i;
align_fsm_reset(e1i);
@@ -290,7 +290,7 @@ * \returns 0 on success, negative on error */ int osmo_e1f_pull_tx_frame(struct osmo_e1f_instance *e1i, uint8_t *out_frame) { - int i; + unsigned int i;
if (e1i->tx.ais) { memset(out_frame, 0xff, 32); @@ -684,7 +684,7 @@ */ int osmo_e1f_rx_frame(struct osmo_e1f_instance *e1i, const uint8_t *in_frame) { - int i; + unsigned int i;
e1_rx_update_crc4(e1i, in_frame);
diff --git a/src/mux_demux.c b/src/mux_demux.c index a588829..efd7ac8 100644 --- a/src/mux_demux.c +++ b/src/mux_demux.c @@ -87,7 +87,7 @@ if (!ts->hdlc.tx_len) { rv = recv(ts->fd, ts->hdlc.tx_buf, sizeof(ts->hdlc.tx_buf), MSG_TRUNC); if (rv > 0) { - if (rv > sizeof(ts->hdlc.tx_buf)) { + if (rv > (int)sizeof(ts->hdlc.tx_buf)) { LOGPTS(ts, DXFR, LOGL_ERROR, "Truncated message: Client tried to " "send %d bytes but our buffer is limited to %lu\n", rv, sizeof(ts->hdlc.tx_buf)); @@ -151,7 +151,7 @@ LOGPTS(ts, DE1D, LOGL_ERROR, "dead socket during read: %s\n", strerror(errno)); e1_ts_stop(ts); - } else if (l < len) { + } else if (l < (int)len) { LOGPTS(ts, DE1D, LOGL_NOTICE, "TS read underflow: We had %zu bytes to read, " "but socket returned only %d\n", len, l); } @@ -297,7 +297,7 @@ LOGPTS(ts, DE1D, LOGL_ERROR, "dead socket during write: %s\n", strerror(errno)); e1_ts_stop(ts); - } else if (rv < len) { + } else if (rv < (int)len) { LOGPTS(ts, DE1D, LOGL_NOTICE, "TS write overflow: We had %zu bytes to send, " "but write returned only %d\n", len, rv); } diff --git a/src/proto_srv.c b/src/proto_srv.c index c376423..d6d3d17 100644 --- a/src/proto_srv.c +++ b/src/proto_srv.c @@ -94,7 +94,7 @@
/* Check payload length */ if ((h->payload_len >= 0) && - (h->payload_len != (msgb_length(msgb) - sizeof(struct osmo_e1dp_msg_hdr)))) + (h->payload_len != (int)(msgb_length(msgb) - sizeof(struct osmo_e1dp_msg_hdr)))) { LOGP(DE1D, LOGL_ERROR, "Invalid payload for message type: %d / (%d/%d/%d).\n", hdr->type, hdr->intf, hdr->line, hdr->ts); diff --git a/src/usb.c b/src/usb.c index d5107bc..32a249a 100644 --- a/src/usb.c +++ b/src/usb.c @@ -189,7 +189,7 @@ flow->cb(flow, libusb_get_iso_packet_buffer_simple(xfr, j), (xfr->iso_packet_desc[j].status == LIBUSB_TRANSFER_COMPLETED) ? - xfr->iso_packet_desc[j].actual_length : -1 + (int)xfr->iso_packet_desc[j].actual_length : -1 ); len += (xfr->iso_packet_desc[j].length = flow->size); } @@ -359,7 +359,7 @@
switch (irq->type) { case ICE1USB_IRQ_T_ERRCNT: - if (xfer->actual_length < sizeof(*irq)) { + if (xfer->actual_length < (int)sizeof(*irq)) { LOGPLI(line, DE1D, LOGL_ERROR, "Short ERRCNT interrupt: %u<%zu\n", xfer->actual_length, sizeof(*irq)); break; diff --git a/src/vty.c b/src/vty.c index 563950d..2f2425c 100644 --- a/src/vty.c +++ b/src/vty.c @@ -107,7 +107,7 @@
static void vty_dump_line(struct vty *vty, const struct e1_line *line) { - int tn; + unsigned int tn;
vty_out(vty, "Interface #%u, Line #%u, Mode %s%s%s:%s", line->intf->id, line->id, get_value_string(e1_line_mode_names, line->mode),