From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f176.google.com (mail-pd0-f176.google.com [209.85.192.176]) by dpdk.org (Postfix) with ESMTP id 971297DFF for ; Fri, 26 Dec 2014 16:28:10 +0100 (CET) Received: by mail-pd0-f176.google.com with SMTP id r10so13178948pdi.7 for ; Fri, 26 Dec 2014 07:28:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=IS8mXrZA0fYo7gKj89ebjrKkYAn2njkJfpmwhT7d7dw=; b=WuXZSkUaHK39v4EDUZzWWfYsBFSF9nDV/cbjqwbALVC8FRn7oxq9eLUZkBpHaDRk2G JtnhitE097nud3aHozkqzAKfH25gu8FBFF0foVgGJkHsRr6kGCA5zW05rojnqlCW1COM cfdD99oWdPnNA78oDNouOzwbNIfJjL8n+LY5Fuf0+iqjWBEe6Wwxv3B5mudOxvIbJOVd 3G90ODOBPVU1mY1qOiOrKuv6AsQyezRjmOF1dRXkpKpoe2gSl6aH/yMlXmwAERqoKYGX +1811IhrM4LbGQR6hNMRxP6hG9+G3y+Emt7wV0hasCpvkOnssQCBsLpexo/W5T+CKL4X CtUg== MIME-Version: 1.0 X-Received: by 10.70.140.167 with SMTP id rh7mr69802148pdb.108.1419607689742; Fri, 26 Dec 2014 07:28:09 -0800 (PST) Received: by 10.70.114.233 with HTTP; Fri, 26 Dec 2014 07:28:09 -0800 (PST) In-Reply-To: <20141226144000.GC5567@localhost.localdomain> References: <1419521597-31978-1-git-send-email-rkerur@gmail.com> <1419521597-31978-3-git-send-email-rkerur@gmail.com> <20141225173041.GD3199@localhost.localdomain> <20141226144000.GC5567@localhost.localdomain> Date: Fri, 26 Dec 2014 07:28:09 -0800 Message-ID: From: Ravi Kerur To: Neil Horman Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 2/7] Move EAL common functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Dec 2014 15:28:11 -0000 On Fri, Dec 26, 2014 at 6:40 AM, Neil Horman wrote: > On Thu, Dec 25, 2014 at 11:23:12AM -0800, Ravi Kerur wrote: > > Thanks Neil for reviews. Inline > > > > On Thu, Dec 25, 2014 at 9:30 AM, Neil Horman > wrote: > > > > > On Thu, Dec 25, 2014 at 10:33:12AM -0500, Ravi Kerur wrote: > > > > eal_debug.c has no difference between Linux and BSD, move > > > > into common directory. > > > > Rename eal_debug.c to eal_common_debug.c > > > > Makefile changes to reflect file move and name change. > > > > Fix checkpatch warnings. > > > > > > > > Signed-off-by: Ravi Kerur > > > > + > > > > +/* not implemented in this environment */ > > > > +void rte_dump_registers(void) > > > > +{ > > > > +} > > > Clearly this function has no use, instead of keeping it around, can you > > > please > > > remove it until someone works up the gumption to make it do something. > > > We're > > > just wasting an extra call instruction here so someone doesn't have to > > > write a > > > prototype in the future. I don't see the value. > > > > > > > This is existing code, I just removed "return" statement as per > > checkpatch. Should I make it "inline" and add a comment indicating to > > revisit whether to make it inline/no inline when the function is > > implemented? > > > I understand its existing code, I'm saying that, while you're moving it > around, > clean it up. Don't make it inline, just remove it, since it does > nothing. If > you feel its important to keep around, I suppose you can make it inline, > but I > don't really think its needed at all. > > Neil > > Sure will remove it. > > > > > > > +/* > > > > + * Like rte_panic this terminates the application. However, no > > > traceback is > > > > + * provided and no core-dump is generated. > > > > + */ > > > > +void > > > > +rte_exit(int exit_code, const char *format, ...) > > > > +{ > > > > + va_list ap; > > > > + > > > > + /* disable history */ > > > > + rte_log_set_history(0); > > > > + > > > > + if (exit_code != 0) > > > > + RTE_LOG(CRIT, EAL, "Error - exiting with code: %d\n" > > > > + " Cause: ", exit_code); > > > > + > > > > + va_start(ap, format); > > > > + rte_vlog(RTE_LOG_CRIT, RTE_LOGTYPE_EAL, format, ap); > > > > + va_end(ap); > > > > + > > > > +#ifndef RTE_EAL_ALWAYS_PANIC_ON_ERROR > > > > + exit(exit_code); > > > > +#else > > > > + rte_dump_stack(); > > > > + rte_dump_registers(); > > > > + abort(); > > > > +#endif > > > This doesn't match with the commentary above. If rte_exit isn't meant > to > > > provide a traceback, it shouldn't do so. If an application wants that > to > > > happen, then they need to use rte_panic. > > > > > > This is again existing code. I can change the comment which > matches > > the function, will it work? >