From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [119.145.14.65]) by dpdk.org (Postfix) with ESMTP id C10707F45 for ; Wed, 29 Oct 2014 06:05:46 +0100 (CET) Received: from 172.24.2.119 (EHLO szxeml460-hub.china.huawei.com) ([172.24.2.119]) by szxrg02-dlp.huawei.com (MOS 4.3.7-GA FastPath queued) with ESMTP id CBN10274; Wed, 29 Oct 2014 13:14:14 +0800 (CST) Received: from [127.0.0.1] (10.177.19.115) by szxeml460-hub.china.huawei.com (10.82.67.203) with Microsoft SMTP Server id 14.3.158.1; Wed, 29 Oct 2014 13:14:08 +0800 Message-ID: <5450779E.1030400@huawei.com> Date: Wed, 29 Oct 2014 13:14:06 +0800 From: Linhaifeng User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 MIME-Version: 1.0 To: Matthew Hall , "Qiu, Michael" References: <1414551269-5820-1-git-send-email-haifeng.lin@huawei.com> <533710CFB86FA344BFBF2D6802E60286C7CAAB@SHSMSX101.ccr.corp.intel.com> <20141029034437.GA29486@mhcomputing.net> In-Reply-To: <20141029034437.GA29486@mhcomputing.net> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.19.115] X-CFilter-Loop: Reflected Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] add free hugepage function 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: Wed, 29 Oct 2014 05:05:48 -0000 On 2014/10/29 11:44, Matthew Hall wrote: > On Wed, Oct 29, 2014 at 03:27:58AM +0000, Qiu, Michael wrote: >> I just saw one return path with value '0', and no any other place >> return a negative value, so it is better to be designed as one >> non-return function, >> >> +void >> +rte_eal_hugepage_free(void) >> +{ >> + struct hugepage_file *hugepg_tbl = g_hugepage_table.hugepg_tbl; >> + unsigned i; >> + unsigned nr_hugefiles = g_hugepage_table.nr_hugefiles; >> + >> + RTE_LOG(INFO, EAL, "unlink %u hugepage files\n", nr_hugefiles); >> + >> + for (i = 0; i < nr_hugefiles; i++) { >> + unlink(hugepg_tbl[i].filepath); >> + hugepg_tbl[i].orig_va = NULL; >> + } >> +} >> + >> >> Thanks, >> Michael > > Actually, I don't think that's quite right. > > http://linux.die.net/man/2/unlink > > "On success, zero is returned. On error, -1 is returned, and errno is set > appropriately." So it should be returning an error, and logging a message for > a file it cannot unlink or people will be surprised with weird failures. > > It also had some minor typos / English in the comments but we can fix that too. > > Matthew. > > Thank you Michael & Matthew I will fix it. :) -- Regards, Haifeng