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 B0783C316 for ; Mon, 25 May 2015 13:53:32 +0200 (CEST) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1Ywr19-0007MP-Es; Mon, 25 May 2015 13:58:08 +0200 Message-ID: <55630D4B.40708@6wind.com> Date: Mon, 25 May 2015 13:53:47 +0200 From: Olivier MATZ User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: Stephen Hemminger , dev@dpdk.org References: <1431707860-19562-1-git-send-email-stephen@networkplumber.org> In-Reply-To: <1431707860-19562-1-git-send-email-stephen@networkplumber.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] eal devargs: don't call rte_log when not initialized 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, 25 May 2015 11:53:32 -0000 Hi Stephen, On 05/15/2015 06:37 PM, Stephen Hemminger wrote: > This problem was discovered when passing invalid PCI id to the > blacklist API in devargs. > > Any failures in rte_devargs_add would cause a core dump because > it would call rte_log() before the the EAL log environment was > initailized. Rather than try and log just remove the messages > and leave it up to the caller to check the return value. > > Most of the other failure possibilities are when malloc() fails, and if > that happens any logging that used malloc() would also fail. > > This failure was not caught by the standalone tests to devargs > because the tests are run after calling rte_eal_init (which is not > how devargs is intended to be used). > > Signed-off-by: Stephen Hemminger Acked-by: Olivier Matz Thanks, Olivier > --- > lib/librte_eal/common/eal_common_devargs.c | 30 +++++++++++++----------------- > 1 file changed, 13 insertions(+), 17 deletions(-) > > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c > index 615945e..ec56165 100644 > --- a/lib/librte_eal/common/eal_common_devargs.c > +++ b/lib/librte_eal/common/eal_common_devargs.c > @@ -31,11 +31,15 @@ > */ > > /* This file manages the list of devices and their arguments, as given > - * by the user at startup */ > + * by the user at startup > + * > + * Code here should not call rte_log since the EAL environment > + * may not be initialized. > + */ > > +#include > #include > > -#include > #include > #include > #include "eal_private.h" > @@ -54,11 +58,8 @@ rte_eal_parse_devargs_str(const char *devargs_str, > return -1; > > *drvname = strdup(devargs_str); > - if (drvname == NULL) { > - RTE_LOG(ERR, EAL, > - "cannot allocate temp memory for driver name\n"); > + if (drvname == NULL) > return -1; > - } > > /* set the first ',' to '\0' to split name and arguments */ > sep = strchr(*drvname, ','); > @@ -70,8 +71,6 @@ rte_eal_parse_devargs_str(const char *devargs_str, > } > > if (*drvargs == NULL) { > - RTE_LOG(ERR, EAL, > - "cannot allocate temp memory for driver arguments\n"); > free(*drvname); > return -1; > } > @@ -88,10 +87,9 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) > > /* use malloc instead of rte_malloc as it's called early at init */ > devargs = malloc(sizeof(*devargs)); > - if (devargs == NULL) { > - RTE_LOG(ERR, EAL, "cannot allocate devargs\n"); > + if (devargs == NULL) > goto fail; > - } > + > memset(devargs, 0, sizeof(*devargs)); > devargs->type = devtype; > > @@ -103,19 +101,17 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) > case RTE_DEVTYPE_BLACKLISTED_PCI: > /* try to parse pci identifier */ > if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 && > - eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0) { > - RTE_LOG(ERR, EAL, "invalid PCI identifier <%s>\n", buf); > + eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0) > goto fail; > - } > + > break; > case RTE_DEVTYPE_VIRTUAL: > /* save driver name */ > ret = snprintf(devargs->virtual.drv_name, > sizeof(devargs->virtual.drv_name), "%s", buf); > - if (ret < 0 || ret >= (int)sizeof(devargs->virtual.drv_name)) { > - RTE_LOG(ERR, EAL, "driver name too large: <%s>\n", buf); > + if (ret < 0 || ret >= (int)sizeof(devargs->virtual.drv_name)) > goto fail; > - } > + > break; > } > >