Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1407?usp=email
to review the following change.
Change subject: packet_id: Avoid conversion warnings
......................................................................
packet_id: Avoid conversion warnings
Most of these already have checks that the
values are as expected, so we can just add
the required casts.
Only the circ list x_sizeof is changed to
size_t since that is its purpose (and it is
not actually used anyway).
Change-Id: Ib0584e8728701cca10ac5675c9cb0e6f5eb901ac
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/circ_list.h
M src/openvpn/packet_id.c
M src/openvpn/packet_id.h
3 files changed, 21 insertions(+), 37 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/07/1407/1
diff --git a/src/openvpn/circ_list.h b/src/openvpn/circ_list.h
index eebf087..12f2bea 100644
--- a/src/openvpn/circ_list.h
+++ b/src/openvpn/circ_list.h
@@ -33,7 +33,7 @@
int x_head; \
int x_size; \
int x_cap; \
- int x_sizeof; \
+ size_t x_sizeof; \
type x_list[]; \
}
@@ -58,14 +58,14 @@
(obj)->x_size = 0; \
}
-#define CIRC_LIST_ALLOC(dest, list_type, size)
\
- {
\
- const int so = sizeof(list_type) + sizeof((dest)->x_list[0]) * (size);
\
- (dest) = (list_type *)malloc(so);
\
- check_malloc_return(dest);
\
- memset((dest), 0, so);
\
- (dest)->x_cap = size;
\
- (dest)->x_sizeof = so;
\
+#define CIRC_LIST_ALLOC(dest, list_type, size)
\
+ {
\
+ const size_t so = sizeof(list_type) + sizeof((dest)->x_list[0]) *
(size); \
+ (dest) = (list_type *)malloc(so);
\
+ check_malloc_return(dest);
\
+ memset((dest), 0, so);
\
+ (dest)->x_cap = size;
\
+ (dest)->x_sizeof = so;
\
}
#define CIRC_LIST_FREE(dest) free(dest)
diff --git a/src/openvpn/packet_id.c b/src/openvpn/packet_id.c
index 08d9d9b..12525d1 100644
--- a/src/openvpn/packet_id.c
+++ b/src/openvpn/packet_id.c
@@ -71,11 +71,6 @@
#endif
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
static void
packet_id_init_recv(struct packet_id_rec *rec, int seq_backtrack, int
time_backtrack,
const char *name, int unit)
@@ -119,7 +114,7 @@
/* Reinitalise the source */
CLEAR(*src);
- packet_id_init_recv(src, dest->seq_backtrack, dest->time_backtrack,
dest->name, dest->unit);
+ packet_id_init_recv(src, (int)dest->seq_backtrack, dest->time_backtrack,
dest->name, dest->unit);
}
void
@@ -170,7 +165,7 @@
diff = p->id - pin->id;
if (diff < CIRC_LIST_SIZE(p->seq_list) && local_now > SEQ_EXPIRED)
{
- CIRC_LIST_ITEM(p->seq_list, diff) = local_now;
+ CIRC_LIST_ITEM(p->seq_list, (int)diff) = local_now;
}
}
else
@@ -264,7 +259,7 @@
}
{
- const time_t v = CIRC_LIST_ITEM(p->seq_list, diff);
+ const time_t v = CIRC_LIST_ITEM(p->seq_list, (int)diff);
if (v == 0)
{
return true;
@@ -386,7 +381,7 @@
return false;
}
- const packet_id_type net_id = htonpid(p->id);
+ const packet_id_type net_id = htonpid((packet_id_type)p->id);
const net_time_t net_time = htontime(p->time);
if (prepend)
{
@@ -640,7 +635,7 @@
p->seq_backtrack, p->time_backtrack, p->max_backtrack_stat,
(int)p->initialized);
if (sl != NULL)
{
- buf_printf(&out, " sl=[%d,%d,%d,%d]", sl->x_head, sl->x_size,
sl->x_cap, sl->x_sizeof);
+ buf_printf(&out, " sl=[%d,%d,%d,%zu]", sl->x_head, sl->x_size,
sl->x_cap, sl->x_sizeof);
}
@@ -655,7 +650,6 @@
{
uint64_t packet_id;
-
if (!buf_read(buf, &packet_id, sizeof(packet_id)))
{
return 0;
@@ -663,16 +657,12 @@
uint64_t id = ntohll(packet_id);
/* top most 16 bits */
- uint16_t epoch = id >> 48;
+ uint16_t epoch = (uint16_t)(id >> 48);
pin->id = id & PACKET_ID_MASK;
return epoch;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
bool
packet_id_write_epoch(struct packet_id_send *p, uint16_t epoch, struct buffer
*buf)
{
diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h
index e9d3647..cdc734d 100644
--- a/src/openvpn/packet_id.h
+++ b/src/openvpn/packet_id.h
@@ -280,26 +280,20 @@
return p->fd >= 0;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
-/* transfer packet_id -> packet_id_persist */
+/* transfer packet_id -> packet_id_persist
+ Note: Only supported for non-epoch format (i.e. 32bit ID, not 64bit format),
+ must be enforced at the caller.
+ */
static inline void
packet_id_persist_save_obj(struct packet_id_persist *p, const struct packet_id
*pid)
{
- if (packet_id_persist_enabled(p) && pid->rec.time)
+ if (packet_id_persist_enabled(p) && pid->rec.time && pid->rec.id <=
PACKET_ID_MAX)
{
p->time = pid->rec.time;
- p->id = pid->rec.id;
+ p->id = (packet_id_type)pid->rec.id;
}
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
/**
* Reset the current send packet id to its initial state.
* Use very carefully (e.g. in the standalone reset packet context) to
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1407?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ib0584e8728701cca10ac5675c9cb0e6f5eb901ac
Gerrit-Change-Number: 1407
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
