Patchwork osutil: fix potential wrong fd close

login
register
mail settings
Submitter Jun Wu
Date March 16, 2017, 3:56 a.m.
Message ID <ebcce969e076146ae9aa.1489636616@localhost.localdomain>
Download mbox | patch
Permalink /patch/19373/
State Accepted
Headers show

Comments

Jun Wu - March 16, 2017, 3:56 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1489635792 25200
#      Wed Mar 15 20:43:12 2017 -0700
# Node ID ebcce969e076146ae9aa3690c35373123f5f508d
# Parent  fb1b5cd17664218f73ed6ba604973b817932593e
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r ebcce969e076
osutil: fix potential wrong fd close

According to POSIX closedir [1]:

  If a file descriptor is used to implement type DIR, that file descriptor
  shall be closed.

According to POSIX fdopendir [2]:

  Upon calling closedir() the file descriptor shall be closed.

So we should avoid "close(dfd)" after "closedir(dir)". With threads, there
could be a race where an innocent fd gets closed. But Python GIL seems to
help hiding the issue well.

[1]: http://pubs.opengroup.org/onlinepubs/009695399/functions/closedir.html
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html
Yuya Nishihara - March 17, 2017, 12:52 p.m.
On Wed, 15 Mar 2017 20:56:56 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1489635792 25200
> #      Wed Mar 15 20:43:12 2017 -0700
> # Node ID ebcce969e076146ae9aa3690c35373123f5f508d
> # Parent  fb1b5cd17664218f73ed6ba604973b817932593e
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r ebcce969e076
> osutil: fix potential wrong fd close

Looks good, queued, thanks.

Patch

diff --git a/mercurial/osutil.c b/mercurial/osutil.c
--- a/mercurial/osutil.c
+++ b/mercurial/osutil.c
@@ -401,4 +401,6 @@  error:
 error_list:
 	closedir(dir);
+	/* closedir also closes its dirfd */
+	goto error_value;
 error_dir:
 #ifdef AT_SYMLINK_NOFOLLOW