From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 75D4829C6 for ; Mon, 25 Feb 2019 11:59:20 +0100 (CET) Received: from lfbn-1-5920-128.w90-110.abo.wanadoo.fr ([90.110.126.128] helo=droids-corp.org) by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1gyE0k-0001RK-Cp; Mon, 25 Feb 2019 12:01:27 +0100 Received: by droids-corp.org (sSMTP sendmail emulation); Mon, 25 Feb 2019 11:59:14 +0100 Date: Mon, 25 Feb 2019 11:59:14 +0100 From: Olivier Matz To: Gage Eads Cc: dev@dpdk.org, arybchenko@solarflare.com, bruce.richardson@intel.com, konstantin.ananyev@intel.com, gavin.hu@arm.com, Honnappa.Nagarahalli@arm.com, nd@arm.com, thomas@monjalon.net Message-ID: <20190225105914.2zsmvp2ighvsbeqi@platinum> References: <20190222160655.3346-1-gage.eads@intel.com> <20190222160655.3346-4-gage.eads@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190222160655.3346-4-gage.eads@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH 3/7] test/stack: add stack test X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Feb 2019 10:59:20 -0000 On Fri, Feb 22, 2019 at 10:06:51AM -0600, Gage Eads wrote: > stack_autotest performs positive and negative testing of the stack API, and > exercises the push and pop datapath functions with all available lcores. > > Signed-off-by: Gage Eads [...] > --- /dev/null > +++ b/test/test/test_stack.c > @@ -0,0 +1,394 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright(c) 2019 Intel Corporation > + */ > + > +#include > + > +#include > +#include > +#include > +#include > + > +#include "test.h" > + > +#define STACK_SIZE 4096 > +#define MAX_BULK 32 > + > +static int > +test_stack_push_pop(struct rte_stack *s, void **obj_table, unsigned int bulk_sz) > +{ > + void *popped_objs[STACK_SIZE]; > + unsigned int i, ret; Here, a dynamic sized table is used. In test_stack_basic() below, it uses a heap-based allocation for the same purpose. I think it would be more consistent to have the same method for both. I suggest to allocate in heap to avoid a stack overflow if STACK_SIZE is increased in the future. [...] > +static int > +test_stack_basic(void) > +{ > + struct rte_stack *s = NULL; > + void **obj_table = NULL; > + int i, ret = -1; > + > + obj_table = rte_calloc(NULL, STACK_SIZE, sizeof(void *), 0); > + if (obj_table == NULL) { > + printf("[%s():%u] failed to calloc %lu bytes\n", > + __func__, __LINE__, STACK_SIZE * sizeof(void *)); > + goto fail_test; > + } > + > + for (i = 0; i < STACK_SIZE; i++) > + obj_table[i] = (void *)(uintptr_t)i; > + > + s = rte_stack_create(__func__, STACK_SIZE, rte_socket_id(), 0); > + if (s == NULL) { > + printf("[%s():%u] failed to create a stack\n", > + __func__, __LINE__); > + goto fail_test; > + } > + > + if (rte_stack_lookup(__func__) != s) { > + printf("[%s():%u] failed to lookup a stack\n", > + __func__, __LINE__); > + goto fail_test; > + } > + > + if (rte_stack_count(s) != 0) { > + printf("[%s():%u] stack count: %u (expected 0)\n", > + __func__, __LINE__, rte_stack_count(s)); > + goto fail_test; > + } > + > + if (rte_stack_free_count(s) != STACK_SIZE) { > + printf("[%s():%u] stack free count: %u (expected %u)\n", > + __func__, __LINE__, rte_stack_count(s), STACK_SIZE); > + goto fail_test; > + } > + > + ret = test_stack_push_pop(s, obj_table, 1); > + if (ret) { > + printf("[%s():%u] Single object push/pop failed\n", > + __func__, __LINE__); > + goto fail_test; > + } > + > + ret = test_stack_push_pop(s, obj_table, MAX_BULK); > + if (ret) { > + printf("[%s():%u] Bulk object push/pop failed\n", > + __func__, __LINE__); > + goto fail_test; > + } > + > + ret = rte_stack_push(s, obj_table, 2 * STACK_SIZE); > + if (ret != 0) { > + printf("[%s():%u] Excess objects push succeeded\n", > + __func__, __LINE__); > + goto fail_test; > + } > + > + ret = rte_stack_pop(s, obj_table, 1); > + if (ret != 0) { > + printf("[%s():%u] Empty stack pop succeeded\n", > + __func__, __LINE__); > + goto fail_test; > + } > + > + ret = 0; > + > +fail_test: > + rte_stack_free(s); > + > + if (obj_table != NULL) > + rte_free(obj_table); > + The if can be removed. > +static int > +test_stack_name_length(void) > +{ > + char name[RTE_STACK_NAMESIZE + 1]; > + struct rte_stack *s; > + > + memset(name, 's', sizeof(name)); > + > + s = rte_stack_create(name, STACK_SIZE, rte_socket_id(), 0); > + if (s != NULL) { > + printf("[%s():%u] Failed to prevent long name\n", > + __func__, __LINE__); > + return -1; > + } Here, "name" is not a valid string (no \0 at the end). It does not hurt because the length check is properly done in the lib, but we could imagine that the wrong name is logged by the library on error, which would trigger a crash here. So I suggest to pass a valid string instead.