Patchwork D1313: dirstate: gate access to self._map.dirs

login
register
mail settings
Submitter phabricator
Date Nov. 3, 2017, 10:59 p.m.
Message ID <differential-rev-PHID-DREV-aezsiykwnn4xxbgahovn-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/25382/
State New
Headers show

Comments

phabricator - Nov. 3, 2017, 10:59 p.m.
mbolin created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  `_droppath()` already checks `"dirs" in self._map.__dict__` before accessing
  `self._map.dirs`. This updates `_addpath()` to be equally cautious.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1313

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS




To: mbolin, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 3, 2017, 11:16 p.m.
mbthomas requested changes to this revision.
mbthomas added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dirstate.py:405
>              scmutil.checkfilename(f)
> -            if f in self._map.dirs:
> -                raise error.Abort(_('directory %r already in dirstate') % f)
> -            # shadows
> -            for d in util.finddirs(f):
> -                if d in self._map.dirs:
> -                    break
> -                entry = self._map.get(d)
> -                if entry is not None and entry[0] != 'r':
> -                    raise error.Abort(
> -                        _('file %r in dirstate clashes with %r') % (d, f))
> +            if "dirs" in self._map.__dict__:
> +                if f in self._map.dirs:

I don't think it's safe to do this.  We're looking for clashes between files and directories, so we do need to look in self.dirs, even if that means generating it because we're accessing it for the first time.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1313

To: mbolin, #hg-reviewers, mbthomas
Cc: mbthomas, durham, mercurial-devel

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -388,7 +388,7 @@ 
         return self._map.copymap
 
     def _droppath(self, f):
-        if self[f] not in "?r" and "dirs" in self._map.__dict__:
+        if "dirs" in self._map.__dict__ and self[f] not in "?r":
             self._map.dirs.delpath(f)
 
         if "filefoldmap" in self._map.__dict__:
@@ -402,16 +402,17 @@ 
         oldstate = self[f]
         if state == 'a' or oldstate == 'r':
             scmutil.checkfilename(f)
-            if f in self._map.dirs:
-                raise error.Abort(_('directory %r already in dirstate') % f)
-            # shadows
-            for d in util.finddirs(f):
-                if d in self._map.dirs:
-                    break
-                entry = self._map.get(d)
-                if entry is not None and entry[0] != 'r':
-                    raise error.Abort(
-                        _('file %r in dirstate clashes with %r') % (d, f))
+            if "dirs" in self._map.__dict__:
+                if f in self._map.dirs:
+                    raise error.Abort(_('directory %r already in dirstate') % f)
+                # shadows
+                for d in util.finddirs(f):
+                    if d in self._map.dirs:
+                        break
+                    entry = self._map.get(d)
+                    if entry is not None and entry[0] != 'r':
+                        raise error.Abort(
+                            _('file %r in dirstate clashes with %r') % (d, f))
         if oldstate in "?r" and "dirs" in self._map.__dict__:
             self._map.dirs.addpath(f)
         self._dirty = True