Most failures of packet_get() indicate a serious problem, and log messages
accordingly. However, a few callers expect failures here, because they're
probing for a certain range which might or might not be in a packet. They
use packet_get_try() which passes a NULL func to packet_get_do() to
suppress the logging which is unwanted in this case.
However, this doesn't just suppress the log when packet_get_do() finds the
requested region isn't in the packet. It suppresses logging for all other
errors too, which do indicate serious problems, even for the callers of
packet_get_try(). Worse it will pass the NULL func on to
packet_check_range() which doesn't expect it, meaning we'll get unhelpful
messages from there if there is a failure.
Fix this by making packet_get_try_do() the primary function which doesn't
log for the case of a range outside the packet. packet_get_do() becomes a
trivial wrapper around that which logs a message if packet_get_try_do()
returns NULL.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
packet.c | 51 +++++++++++++++++++++++++++++++++++----------------
packet.h | 8 +++++---
2 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/packet.c b/packet.c
index 7cbe95da..b3e8c79e 100644
--- a/packet.c
+++ b/packet.c
@@ -89,7 +89,7 @@ bool pool_full(const struct pool *p)
* @p: Existing pool
* @len: Length of new descriptor
* @start: Start of data
- * @func: For tracing: name of calling function, NULL means no trace()
+ * @func: For tracing: name of calling function
* @line: For tracing: caller line of function call
*/
void packet_add_do(struct pool *p, size_t len, const char *start,
@@ -113,39 +113,31 @@ void packet_add_do(struct pool *p, size_t len, const char *start,
}
/**
- * packet_get_do() - Get data range from packet descriptor from given pool
+ * packet_get_try_do() - Get data range from packet descriptor from given pool
* @p: Packet pool
* @idx: Index of packet descriptor in pool
* @offset: Offset of data range in packet descriptor
* @len: Length of desired data range
* @left: Length of available data after range, set on return, can be NULL
- * @func: For tracing: name of calling function, NULL means no trace()
+ * @func: For tracing: name of calling function
* @line: For tracing: caller line of function call
*
* Return: pointer to start of data range, NULL on invalid range or descriptor
*/
-void *packet_get_do(const struct pool *p, size_t idx, size_t offset,
- size_t len, size_t *left, const char *func, int line)
+void *packet_get_try_do(const struct pool *p, size_t idx, size_t offset,
+ size_t len, size_t *left, const char *func, int line)
{
char *ptr;
if (idx >= p->size || idx >= p->count) {
- if (func) {
- trace("packet %zu from pool size: %zu, count: %zu, "
- "%s:%i", idx, p->size, p->count, func, line);
- }
+ trace("packet %zu from pool size: %zu, count: %zu, %s:%i",
+ idx, p->size, p->count, func, line);
return NULL;
}
if (offset > p->pkt[idx].iov_len ||
- len > (p->pkt[idx].iov_len - offset)) {
- if (func) {
- trace("data length %zu, offset %zu from length %zu, "
- "%s:%i", len, offset, p->pkt[idx].iov_len,
- func, line);
- }
+ len > (p->pkt[idx].iov_len - offset))
return NULL;
- }
ptr = (char *)p->pkt[idx].iov_base + offset;
@@ -158,6 +150,33 @@ void *packet_get_do(const struct pool *p, size_t idx, size_t offset,
return ptr;
}
+/**
+ * packet_get_do() - Get data range from packet descriptor from given pool
+ * @p: Packet pool
+ * @idx: Index of packet descriptor in pool
+ * @offset: Offset of data range in packet descriptor
+ * @len: Length of desired data range
+ * @left: Length of available data after range, set on return, can be NULL
+ * @func: For tracing: name of calling function
+ * @line: For tracing: caller line of function call
+ *
+ * Return: as packet_get_try_do() but log a trace message when returning NULL
+ */
+void *packet_get_do(const struct pool *p, const size_t idx,
+ size_t offset, size_t len, size_t *left,
+ const char *func, int line)
+{
+ void *r = packet_get_try_do(p, idx, offset, len, left, func, line);
+
+ if (!r) {
+ trace("missing packet data length %zu, offset %zu from "
+ "length %zu, %s:%i",
+ len, offset, p->pkt[idx].iov_len, func, line);
+ }
+
+ return r;
+}
+
/**
* pool_flush() - Flush a packet pool
* @p: Pointer to packet pool
diff --git a/packet.h b/packet.h
index 9061dad7..c94780a5 100644
--- a/packet.h
+++ b/packet.h
@@ -32,6 +32,9 @@ struct pool {
int vu_packet_check_range(void *buf, const char *ptr, size_t len);
void packet_add_do(struct pool *p, size_t len, const char *start,
const char *func, int line);
+void *packet_get_try_do(const struct pool *p, const size_t idx,
+ size_t offset, size_t len, size_t *left,
+ const char *func, int line);
void *packet_get_do(const struct pool *p, const size_t idx,
size_t offset, size_t len, size_t *left,
const char *func, int line);
@@ -41,12 +44,11 @@ void pool_flush(struct pool *p);
#define packet_add(p, len, start) \
packet_add_do(p, len, start, __func__, __LINE__)
+#define packet_get_try(p, idx, offset, len, left) \
+ packet_get_try_do(p, idx, offset, len, left, __func__, __LINE__)
#define packet_get(p, idx, offset, len, left) \
packet_get_do(p, idx, offset, len, left, __func__, __LINE__)
-#define packet_get_try(p, idx, offset, len, left) \
- packet_get_do(p, idx, offset, len, left, NULL, 0)
-
#define PACKET_POOL_DECL(_name, _size, _buf) \
struct _name ## _t { \
char *buf; \
--
2.48.1