From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f177.google.com (mail-pd0-f177.google.com [209.85.192.177]) by dpdk.org (Postfix) with ESMTP id 8A6C395FE for ; Thu, 25 Dec 2014 20:23:13 +0100 (CET) Received: by mail-pd0-f177.google.com with SMTP id ft15so11871200pdb.22 for ; Thu, 25 Dec 2014 11:23:12 -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=kZLPEokTXMBDBOekLArOWNzLGxZihXdXbUcn1pJEQYc=; b=BJWARtNQIxuYzjFNVpuwbdvqkUokj6ExbsDoNv19nTcUkPdpIWKOFCsCzZnKbp3gn5 GGPKenHWGlEEx0D0hUPquJsQlWeIgfplEth/3EZo5McWmjrAq62iOem9Zt7V+10wHliH ETXkRWwYxWQ4jcwysgKucenFmtRt1Dp0fkJpOehDNSi6t/MZ+NPKjDe8G90Gc9Wulx1Y tS718ghTZFpKTcF4KNIyXJONXzgSpK/P6PNnyPdBVyb3YEEBGJvj7m2QUfmptIamvUR3 7z0kb0FJRXl4FartXQ6tys3GImOzIyHgBp7Lw7LJQhDbbUoepMohc5B8afSiKLtZGMrH Le0A== MIME-Version: 1.0 X-Received: by 10.70.65.105 with SMTP id w9mr63599011pds.58.1419535392820; Thu, 25 Dec 2014 11:23:12 -0800 (PST) Received: by 10.70.114.233 with HTTP; Thu, 25 Dec 2014 11:23:12 -0800 (PST) In-Reply-To: <20141225173041.GD3199@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> Date: Thu, 25 Dec 2014 11:23:12 -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: Thu, 25 Dec 2014 19:23:14 -0000 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? > > > +/* > > + * 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?