From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) by dpdk.org (Postfix) with ESMTP id B3C34593A for ; Mon, 5 Jan 2015 10:41:12 +0100 (CET) Received: by mail-wi0-f175.google.com with SMTP id l15so2866250wiw.8 for ; Mon, 05 Jan 2015 01:41:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=AIENtCIcqi+ARmcFJghVrB6qolBKFyqh8MSTX/ehFOc=; b=KCQY1GYlyz7OFzffypggBBA62IZLuVxVcew1BqDqvD3fXa/SlSfwe1i653JcYHVzvC plXWL12XcY79x6PFKd/dlF0K267O+gC1+8QtMVq8fxUyY0MDGMcrZoEFbU7PiNbAe2Rk YopQvZUv8U+QSBrGgdKZ/LDRF4fx4+p32WVDePo7NbbMBeF1dRkfVKXZ5gRq8TO3iAgt 7soRnJodcTsHSwEaHSfzatYgIGgBJKs2O8sbuL5YhDwy0jtA0SXf5n4XI6Vk2KZj2fy2 9bFCO0+ZWec20QljG4ASxFGeK8K6crZsSvhFDl800nHNsjdU6ijOtnVCFFpPqECPOvEO lS0Q== X-Gm-Message-State: ALoCoQlZ3CQfuXOoLT4JpMMtUEVQIrnfpu0U/CW0EyWK4iMoqHhSTkMHHREaF1AAZJwqLqu8pil8 X-Received: by 10.180.95.136 with SMTP id dk8mr23831570wib.64.1420450872537; Mon, 05 Jan 2015 01:41:12 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id o16sm41244172wjw.7.2015.01.05.01.41.11 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 05 Jan 2015 01:41:11 -0800 (PST) From: Thomas Monjalon To: Ravi Kerur Date: Mon, 05 Jan 2015 10:40:50 +0100 Message-ID: <1568436.hdI4bbmShK@xps13> Organization: 6WIND User-Agent: KMail/4.14.3 (Linux/3.17.6-1-ARCH; KDE/4.14.3; x86_64; ; ) In-Reply-To: References: <1419521597-31978-1-git-send-email-rkerur@gmail.com> <20141226144000.GC5567@localhost.localdomain> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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: Mon, 05 Jan 2015 09:41:12 -0000 2014-12-26 07:28, Ravi Kerur: > 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. Please remove it in a separate patch (before this one). > > > > > +/* > > > > > + * 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? Please do not change anything (except perhaps code style) when moving code. If RTE_EAL_ALWAYS_PANIC_ON_ERROR must be removed (to discuss), it should be done in another patchset. Thanks -- Thomas