Re: [dpdk-dev] [PATCH 2/2] eal: fix not checking unlock result

[Burakov, Anatoly]

On 25-Apr-18 8:09 AM, Tan, Jianfeng wrote:


On 4/17/2018 11:42 PM, Anatoly Burakov wrote:
Coverity issue: 272607

Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
Cc: anatoly.bura...@intel.com

Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
---
  lib/librte_eal/linuxapp/eal/eal_hugepage_info.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c 
b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
index 009f963..01fee51 100644
--- a/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
+++ b/lib/librte_eal/linuxapp/eal/eal_hugepage_info.c
@@ -304,8 +304,11 @@ clear_hugedir(const char * hugedir)
          /* if lock succeeds, unlock and remove the file */
          if (lck_result != -1) {
              lck.l_type = F_UNLCK;
-            fcntl(fd, F_SETLK, &lck);
-            unlinkat(dir_fd, dirent->d_name, 0);
+            if (fcntl(fd, F_SETLK, &lck) < 0)
+                RTE_LOG(ERR, EAL, "Couldn't unlock %s: %s\n",
+                    dirent->d_name, strerror(errno));

It seems that we shall return error if this nearly-impossible error 
happens, no?

I'm not sure if we should. In any case, lock will be dropped by close(), 
so the proper fix would've been just remove the fcntl() call altogether.

I'll respin.


+            else
+                unlinkat(dir_fd, dirent->d_name, 0);
          }
          close (fd);
          dirent = readdir(dir);




--
Thanks,
Anatoly