Commits

JanKanis committed 78c57e7 Draft

Fix tests and changed interfaces

- also add some other things, such as attempting to recognize garbage
reads from the (supposed) inotify fd

- a few more tests

- some minor changes for python 2.7 compatibility

Comments (0)

Files changed (5)

inotify/_inotify.c

 #include <sys/ioctl.h>
 #include <unistd.h>
 
+/* This should be at least large enough to hold a single
+ * inotify_event, otherwise crashes can occur. */
 #define READ_BUF_SIZE 64*1024
 
 /* for older pythons */
 		 __typeof__ (b) _b = (b); \
 	 _a < _b ? _a : _b; })
 
+#define INE_SIZE sizeof(struct inotify_event)
+
 
 static PyObject *init(PyObject *self, PyObject *args)
 {
 		while (pos < size) {
 			struct inotify_event *in = (struct inotify_event *) (buffer + pos);
 
-			if (size - pos < sizeof(struct inotify_event) ||
-					size - pos < sizeof(struct inotify_event) + in->len) {
+			// order these comparisons so there won't be an overflow if in->len is very large
+			if (size - pos < INE_SIZE || size - pos - INE_SIZE < in->len) {
+				if (pos == 0 ||
+						in->len > READ_BUF_SIZE - INE_SIZE ||
+						in->len >= readable - (read_total - nread + pos + INE_SIZE)) {
+					// This is not supposed to happen, unless we are reading
+					// garbage. Maybe the fd wasn't an inotify fd?
+					PyErr_SetString(PyExc_TypeError, "python-inotify internal error: " 
+							"read value from inotify fd seems to be garbage, "
+							"are you sure this is the right fd?");
+					goto bail;
+				}
+				// we read a partial message
 				memcpy(buffer, buffer + pos, size - pos);
 				pos = size - pos;
 				goto nextread;

inotify/pathresolver.py

 
 
 class InvalidPathError (OSError):
-    def __init__(self, msg, path, *args, errno=None):
+    def __init__(self, msg, path, errno=None, *args):
         self.filename = path
         self.errno = errno
         if errno:
 class ConcurrentFilesystemModificationError (InvalidPathError):
     def __init__(self, path, *args):
         msg = "Path not valid: A concurrent change was detected while traversing '{}'".format(path)
-        InvalidPathError.__init__(self, msg, path, *args)
+        InvalidPathError.__init__(self, msg, path, errno=None, *args)
 
 
 # To be Python 2 and 3 compatible and also inherit from the right exception

inotify/pathwatcher.py

 from pathlib import PosixPath
 
 from . import pathresolver
-from . import _inotify
+from . import inotify as _inotify
 from .in_constants import constants, decode_mask, event_properties
 from .watcher import NoFilesException, _make_getter
 from .pathresolver import SymlinkLoopError, ConcurrentFilesystemModificationError
         '''Register a watch to be .reconnect()'ed after event processing is finished'''
         self._reconnect.add(watch)
 
-    def read(self, block=True, bufsize=None):
+    def read(self, block=True):
         '''Read a list of queued inotify events.
 
-        block: If block is false, return only those events that can be read immediately.
-
-        bufsize: The buffer size to use to read events. Only meaningful if
-        block is True. If bufsize is too small, an error will occur.
-
+        block: If block is false, return only those events that can be
+        read immediately.
         '''
 
-        if not block:
-            bufsize = 0
-        elif bufsize == 0:
-            bufsize = None
-
         # We call _inotify.read once at first. If we are expecting an
         # IN_IGNORE, we read it again until we get all pening
         # IN_IGNOREs. Secondly, if a
             do2 = True
             while do2 or self._pending_watch_removes > 0:
                 do2 = False
-                for e in self._read_events(bufsize):
+                for e in self._read_events(block):
                     # only filter duplicate path_changed events to
                     # prevent hiding of bugs that may cause duplicate
                     # other events to be created.
         events, self.events = self.events, []
         return events
 
-    def _read_events(self, bufsize):
-        for evt in _inotify.read(self.fd, bufsize):
+    def _read_events(self, block):
+        for evt in _inotify.read(self.fd, block=block):
             if evt.wd == -1:
                 eventiter = self._handle_descriptorless_event(evt)
             else:
     def _do_reconnect(self):
         # Do not just clear the list, but replace it, because the
         # reconnect call can cause new pathwatches to require
-        # reconnection.
+        # recursive reconnection.
         r, self._reconnect = self._reconnect, set()
         for w in r:
             w.reconnect()
 def test_open(w):
   w.add('testfile', inotify.IN_OPEN | inotify.IN_CLOSE)
   open('testfile').close()
-  ev1, ev2 = w.read(0)
+  ev1, ev2 = w.read(block=False)
   assert ev1.open
   assert ev2.close
   assert ev2.close_nowrite
 
 def test_move(w):
   w.add('.', inotify.IN_MOVE)
-  assert w.read(0) == []
+  assert w.read(block=False) == []
   os.rename('testfile', 'targetfile')
-  ev = w.read(0)
+  ev = w.read(block=False)
   for e in ev:
     if e.name == 'testfile':
       assert e.moved_from
   assert w.get_watch('testfile') == w.get_watch('testlink')
   assert len(w.watches()) == 1
   open('testlink').close()
-  ev1, ev2 = w.read(0)
+  ev1, ev2 = w.read(block=False)
   assert ev1.open and ev2.close
   w.remove_path('testfile')
   open('testlink').close()
-  ev = w.read(0)
+  ev = w.read(block=False)
   assert any(e.close for e in ev)
 
 
 def test_delete(w):
     w.add('testfile', inotify.IN_DELETE_SELF)
     os.remove('testfile')
-    ev1, ev2 = w.read(0)
+    ev1, ev2 = w.read(block=False)
     assert ev1.delete_self
     assert ev2.ignored
     assert w.num_watches() == 0
 
 def test_repr(w):
   w.add_all('.', inotify.IN_ALL_EVENTS)
-  print(w.read(0))
+  print(w.read(block=False))
+
+
+def test_read_args(w):
+  with pytest.raises(watcher.NoFilesException):
+    w.read()
+  w.add('.', inotify.IN_ALL_EVENTS)
+  evts = inotify.inotify.read(w.fileno(), block=False)
+  assert evts == []
+  readfd, writefd = os.pipe()
+  os.write(writefd, b'a'*100)
+  with pytest.raises(TypeError):
+    inotify.inotify.read(readfd, block=False)
+
+  ## In the current implementation this succeeds, but it's not
+  ## guaranteed or anything
+  # os.write(writefd, b'\0'*32)
+  # evts = inotify.inotify.read(readfd, block=False)
+  # for e in evts:
+  #   assert e.wd == e.mask == 0
+  #   assert e.name == e.cookie == None
+
+  os.close(writefd)
+  os.close(readfd)
+  
+
+def test_kwarg(w):
+  with pytest.raises(TypeError):
+    inotify.inotify.read(w.fileno(), False)
 
 
 import inotify
-import inotify._inotify
 
 globals().update(inotify.constants)
 
   occurs due to ConcurrentFilesystemModificationError, and also make
   sure we wait for the creation event and restore the watch.
 
-- Can _inotify.read be interrupted without losing events or losing
+- Can inotify.read be interrupted without losing events or losing
   consistency?
+  ----> only a blocking read can now safely be interrupted, but there
+  should be no specific need to interrupt a non-blocking read.
 
 - Unmounting of a filesystem mounted on a watched directory only
   generates events on the mount point itself, not on the parent
 """
 
 
-# from IPython.terminal.ipapp import TerminalIPythonApp
-from IPython import embed as ipythonembed
-# ipapp = TerminalIPythonApp.instance()
-# ipapp.initialize(argv=[]) # argv=[] instructs IPython to ignore sys.argv
+# from IPython import embed as ipythonembed
 
 
 @pytest.fixture(autouse=True)
 
 
 def test_constants():
-  assert IN_PATH_MOVED_TO > inotify._inotify.IN_ALL_EVENTS
-  c_events = functools.reduce(operator.or_, (v for k,v in inotify._inotify.__dict__.items() if k.startswith('IN_')))
+  assert IN_PATH_MOVED_TO > inotify.inotify.IN_ALL_EVENTS
+  c_events = functools.reduce(operator.or_, (v for k,v in inotify.inotify.__dict__.items() if k.startswith('IN_')))
   path_events = functools.reduce(operator.or_, (v for k,v in inotify.constants.items() if k.startswith('IN_PATH_')))
   assert not c_events & path_events
   assert path_events > c_events
   watch = w._paths[P('link1')]
   assert len(watch.links) == 5
   w1, w2, w3, w4, wt  = watch.links
-  assert [w.name for w in (w1, w2, w3, w4)] == 'link1 link2 link3 testfile'.split()
+  assert [wx.name for wx in (w1, w2, w3, w4)] == 'link1 link2 link3 testfile'.split()
   assert (wt.path, wt.name) == ('testfile', None)
   assert w1.wd == w2.wd == w3.wd == w4.wd
   desc = w1.wd