From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f174.google.com (mail-pf0-f174.google.com [209.85.192.174]) by dpdk.org (Postfix) with ESMTP id 417232B9B for ; Fri, 5 Aug 2016 18:34:05 +0200 (CEST) Received: by mail-pf0-f174.google.com with SMTP id x72so98753288pfd.2 for ; Fri, 05 Aug 2016 09:34:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=OqvWWRUi+F3ojzggtrGcvcmQtM+uN9oMhlemJPbGsFo=; b=kc1HoFU3cJUfeZZd8NJd2ecaH1dg5QBjwLRZ+y9jVCKBHeElW9WctNWYNZ2pgAP4s1 TIXsyuSUxAhBsERfr4f4IC+s6JqQAenlYNJmn2jIP3jxuTDWMFqnvDJBNSYq1hCPyPBY PnQ5IhziITVTTzQ6KXBzXoBcj7t+tD+B2LcOFHwKEEz3yvPMicWf+vP6j+YOqHTe83Xt ViO/7RoBmcP5xW83/Ph+dXJ2fddG9SAumZr91cNqPCzIs0E7K0nZC3mLIZvvYVR9+HsC i3VFyleCuiDZpdeYtfZBoQm/ysuQro+eV9Q/VLd+FDIxiPDIR6ar83dTuXJ6rGyvBOeZ 0N7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=OqvWWRUi+F3ojzggtrGcvcmQtM+uN9oMhlemJPbGsFo=; b=m5+4DmyWYgLUl/F9OI+6vYaPGs7KJYPYzaXvvEZ37KSSsOSLh2pYWII64bTnmpamQs QTPABA8ZtJfwtpqlO4vDj5HgojCfCMa/YeWb7St6nNOQwrGQPPzGR6PidR4d9VF3SrGK BCN6x7MJ5voIeD2VQbCHRemtxN3vvFj8FoXrxV1Oh4Je9y/9DNSs31RTQHNMjchVTMuh fN5diA7aWdpk0MQYCpAWgcGRaB90ziiC87tjtWA6EH6H9OJ2RbZP1xamGX98JTRUGRUG PRndrOZIIxJfudDWyatQVuw/elZYnJ+OYVaT5BAbFY4pw0Y2KtIoIR2mrgUutdijes7Y rJ3g== X-Gm-Message-State: AEkooutgUS6PJOcvW57UuZe+06KCSfuOadf1F7Gz5Iyh8/zFjKO1Y0ZVHyrXBVKq8xyY+g== X-Received: by 10.98.18.221 with SMTP id 90mr138157890pfs.3.1470414844505; Fri, 05 Aug 2016 09:34:04 -0700 (PDT) Received: from xeon-e3 (static-50-53-69-251.bvtn.or.frontiernet.net. [50.53.69.251]) by smtp.gmail.com with ESMTPSA id tm1sm29296847pac.23.2016.08.05.09.34.03 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 05 Aug 2016 09:34:04 -0700 (PDT) Date: Fri, 5 Aug 2016 09:34:13 -0700 From: Stephen Hemminger To: Jianfeng Tan Cc: dev@dpdk.org, yuanhan.liu@linux.intel.com, zhihong.wang@intel.com, lining18@jd.com Message-ID: <20160805093413.431ba323@xeon-e3> In-Reply-To: <1470397003-5782-4-git-send-email-jianfeng.tan@intel.com> References: <1470397003-5782-1-git-send-email-jianfeng.tan@intel.com> <1470397003-5782-4-git-send-email-jianfeng.tan@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 3/3] net/virtio_user: fix dev not freed after init error 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, 05 Aug 2016 16:34:05 -0000 On Fri, 5 Aug 2016 11:36:43 +0000 Jianfeng Tan wrote: > diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c > index daef09b..62ccb0b 100644 > --- a/drivers/net/virtio/virtio_user_ethdev.c > +++ b/drivers/net/virtio/virtio_user_ethdev.c > @@ -313,6 +313,17 @@ virtio_user_eth_dev_alloc(const char *name) > return eth_dev; > } > > +static void > +virtio_user_eth_dev_free(struct rte_eth_dev *eth_dev) > +{ > + struct rte_eth_dev_data *data = eth_dev->data; > + struct virtio_hw *hw = data->dev_private; > + > + rte_free(hw->virtio_user_dev); > + rte_free(hw); > + rte_eth_dev_release_port(eth_dev); > +} > + > /* Dev initialization routine. Invoked once for each virtio vdev at > * EAL init time, see rte_eal_dev_init(). > * Returns 0 on success. > @@ -328,7 +339,7 @@ virtio_user_pmd_devinit(const char *name, const char *params) > uint64_t queue_size = VIRTIO_USER_DEF_Q_SZ; > char *path = NULL; > char *mac_addr = NULL; > - int ret = -1; > + int result = -1, ret; It is not clear why two return value variables are needed? > > if (!params || params[0] == '\0') { > PMD_INIT_LOG(ERR, "arg %s is mandatory for virtio_user", > @@ -411,15 +422,19 @@ virtio_user_pmd_devinit(const char *name, const char *params) > > hw = eth_dev->data->dev_private; > if (virtio_user_dev_init(hw->virtio_user_dev, path, queues, cq, > - queue_size, mac_addr) < 0) > + queue_size, mac_addr) < 0) { > + PMD_INIT_LOG(ERR, "virtio_user_dev_init fails"); > + virtio_user_eth_dev_free(eth_dev); > goto end; > + } > > /* previously called by rte_eal_pci_probe() for physical dev */ > if (eth_virtio_dev_init(eth_dev) < 0) { > PMD_INIT_LOG(ERR, "eth_virtio_dev_init fails"); > + virtio_user_eth_dev_free(eth_dev); > goto end; > } > - ret = 0; > + result = 0; > > end: > if (kvlist) > @@ -428,7 +443,7 @@ end: > free(path); > if (mac_addr) > free(mac_addr); Unrelated, but this code could eliminate those if () tests. > - return ret; > + return result; > }