Hi Shally,
Thx for the review.
My comments below:
-----Original Message-----
From: Tomasz Jozwiak <tjozwia...@gmail.com>
Sent: Friday, June 28, 2019 3:56 AM
To: dev@dpdk.org; fiona.tr...@intel.com; tjozwia...@gmail.com; Shally
Verma <shal...@marvell.com>; arturx.tryb...@intel.com
Subject: [EXT] [PATCH v4 3/6] app/test-compress-perf: add verification test
case
External Email
----------------------------------------------------------------------
From: Tomasz Jozwiak <tomaszx.jozw...@intel.com>
...
diff --git a/app/test-compress-perf/comp_perf_test_verify.c b/app/test-
compress-perf/comp_perf_test_verify.c
index 28a0fe8..c2aab70 100644
--- a/app/test-compress-perf/comp_perf_test_verify.c
+++ b/app/test-compress-perf/comp_perf_test_verify.c
@@ -8,14 +8,48 @@
#include <rte_compressdev.h>
#include "comp_perf_test_verify.h"
+#include "comp_perf_test_common.h"
+
+void
+cperf_verify_test_destructor(void *arg) {
+ if (arg) {
+ comp_perf_free_memory(&((struct cperf_verify_ctx *)arg)-
mem);
+ rte_free(arg);
+ }
+}
+
+void *
+cperf_verify_test_constructor(uint8_t dev_id, uint16_t qp_id,
+ struct comp_test_data *options)
+{
+ struct cperf_verify_ctx *ctx = NULL;
+
+ ctx = rte_malloc(NULL, sizeof(struct cperf_verify_ctx), 0);
+
Better just return from here
[Tomek]. Yes you right. we wasn't able to allocate 'cperf_verify_ctx
struct',
so we don't need to call destructor here. I assume there's the same issue
in benchmark patch - will align both in V5. Thx.
+ if (ctx != NULL) {
+ ctx->mem.dev_id = dev_id;
+ ctx->mem.qp_id = qp_id;
+ ctx->options = options;
+
+ if (!comp_perf_allocate_memory(ctx->options, &ctx->mem)
&&
+ !prepare_bufs(ctx->options, &ctx->mem))
+ return ctx;
What if condition fails on comp_per_allocate_memory(), then it will go to
verify_test_destructor(), so comp_perf_free_memory() check if mem != NULL
before calling actual free?
[Tomek] I mean it's ok. Please take in to account that we was able to
allocate 'cperf_verify_ctx struct' - cause
ctx != NULL here. that means 'mem struct' inside 'cperf_verify_ctx
struct' exists for sure:
struct cperf_verify_ctx {
*struct cperf_mem_resources mem;*
struct comp_test_data *options;
int silent;
size_t comp_data_sz;
size_t decomp_data_sz;
double ratio;
};
and all fields inside 'struct cperf_mem_resources mem' are zeroed.
We don't need to check mem != NULL before free, because in this place
mem != NULL for sure. Also it's ok to call 'rte_free',
'rte_mempool_free' and 'rte_pktmbuf_free' with NULL ptr.
as a argument because the check is inside all of these functions.
Thx for the comments.
--
Tomek
