From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id DF1A9237 for ; Thu, 12 Oct 2017 10:28:04 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Oct 2017 01:28:03 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,365,1503385200"; d="scan'208";a="162276110" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.37]) by fmsmga006.fm.intel.com with SMTP; 12 Oct 2017 01:28:01 -0700 Received: by (sSMTP sendmail emulation); Thu, 12 Oct 2017 09:28:00 +0100 Date: Thu, 12 Oct 2017 09:28:00 +0100 From: Bruce Richardson To: Pavan Nikhilesh Cc: reshma.pattan@intel.com, dev@dpdk.org Message-ID: <20171012082759.GA44304@bricha3-MOBL3.ger.corp.intel.com> References: <1505228764-9738-1-git-send-email-pbhagavatula@caviumnetworks.com> <1505228764-9738-2-git-send-email-pbhagavatula@caviumnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1505228764-9738-2-git-send-email-pbhagavatula@caviumnetworks.com> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.8.3 (2017-05-23) Subject: Re: [dpdk-dev] [PATCH 2/2] test/reorder: fix reorder drain 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: Thu, 12 Oct 2017 08:28:05 -0000 On Tue, Sep 12, 2017 at 08:36:04PM +0530, Pavan Nikhilesh wrote: > The reorder drain test fails due to mempool corruption caused by freeing > packet buffer twice. > > Fixes: d0c9b58d7156 ("app/test: new reorder unit test") > > Signed-off-by: Pavan Nikhilesh > --- > test/test/test_reorder.c | 33 +++++++++++++++++++++++---------- > 1 file changed, 23 insertions(+), 10 deletions(-) > > diff --git a/test/test/test_reorder.c b/test/test/test_reorder.c > index 4ec22ac..51c2dcd 100644 > --- a/test/test/test_reorder.c > +++ b/test/test/test_reorder.c > @@ -70,13 +70,15 @@ test_reorder_create(void) > TEST_ASSERT((b == NULL) && (rte_errno == EINVAL), > "No error on create() with NULL name"); > > - b = rte_reorder_create("PKT", rte_socket_id(), REORDER_BUFFER_SIZE_INVALID); > + b = rte_reorder_create("PKT", rte_socket_id(), > + REORDER_BUFFER_SIZE_INVALID); > TEST_ASSERT((b == NULL) && (rte_errno == EINVAL), > "No error on create() with invalid buffer size param."); > > b = rte_reorder_create("PKT_RO1", rte_socket_id(), REORDER_BUFFER_SIZE); > TEST_ASSERT_EQUAL(b, test_params->b, > - "New reorder instance created with already existing name"); > + "New reorder instance created with already existing" > + " name"); > > return 0; > } These changes are just cosmetic and so shouldn't really be included in this patch. Ideally, cosmetic changes should be made only when you are already touching the affected lines anyway due to some other change. Also, it's bad practice to split literal strings across lines, even if the line ends up crossing the 80-char threshold. Being able to grep error strings is more important than keeping lines short. This applies to many of the other changes in this patch. It makes it hard to review for the actual functional changes. Regards, /Bruce