_inotify should specify result type of all ctypes function calls

[fijal]

PyPy emit RuntimeWarning for this case and not specifying it is very fragile (it might work now, but the general idea seems to be very fragile).

[exarkun]

(In [32078]) Branching to 'inotify-types-decl-5147'

[exarkun]

Declarations fixed in the branch. I expanded an existing unit test to cover the type declarations for all three ctypes-invoked functions. I also fixed a bug in the implementation which apparently let the implementation only work by accident on CPython and prevented it from working on PyPy. This caused a test to fail on PyPy, which apparently has a stricter version of ctypes argument checking logic.

For ease of review, test runs from CPython and the latest PyPy nightly against trunk and the branch are attached. Reviewers can also find PyPy nightlies at < http://buildbot.pypy.org/nightly/trunk/>.

Not bothering to force a build now because half the builders are offline. It would certainly be nice to see the results of building the branch before merging this, though.

[jesstess]

Thanks for working on this, exarkun. Some feedback:

• the docstring for initializeModule should reflect that it is now setting argtypes and restype for all 3 functions
• it looks like inotify_add_watch takes a uint32_t mask and not an int mask as its third argument:

       libc.inotify_add_watch.argtypes = [
           ctypes.c_int, ctypes.c_char_p, ctypes.c_int]
  

• I don't see a failing PyPy test in the attached test run results, so I'm not sure what test now passes. PyPy and Twisted aren't playing nicely on my dev machine right now or I'd check myself, but if a failing test is now passing, great.

Other than that, I think this is good to merge!

Build results  here.

[exarkun]

the docstring for initializeModule should reflect ...

Fixed in r32094

it looks like inotify_add_watch takes a uint32_t mask

Oops! Thanks. Fixed in r32095.

I don't see a failing PyPy test in the attached test run results

Indeed, sorry for not being more clear. The initial purpose of the branch was just to clean up the warnings visible on PyPy. While doing that, I found that if I only added the type definitions, this caused a failure due to the bug in INotify.ignore which would trigger a test failure. So then I went ahead and fixed that bug to get the test passing again.

The details of the test failure, for the curious, were this. ignore would sometimes pass None as the watch id value to inotify_rm_watch. CPython 2.6 (at least) silently translated None to 0, making the call possible but not valid, since no watch id value ever seems to be 0 (but who knows if that actually holds). It would then go on to try to pop None out of a dictionary which failed with a KeyError, which is what the test wanted in the first place.

Once the argument types for inotify_rm_watch were defined, ctypes stopped accepting None - since it had been told the parameter is a c_uint32, it wanted an integer that fit in that range. So the code path started failing with a TypeError.

The fix to the test was to explicitly raise the KeyError sooner in this case, before ever trying to call inotify_rm_watch.

[exarkun]

Oops. It didn't make sense to me that inotify_add_watch would return int representing a new watch id but inotify_rm_watch would take uint32 representing the watch id to remove, so I dug around a little bit. The man page seems broken. The header file more sensibly handles the watch id as an int in both places. Only the mask is a uint32. I'll update the type definitions to match the header file instead of the man page.

[exarkun]

(In [32097]) Merge inotify-types-decl-5147

Author: exarkun Reviewer: jesstess Fixes: #5147

Declare the argument and return types on the ctypes wrapper functions used in twisted.python._inotify. Also fix a case where inotify_rm_watch was incorrectly called with None when INotify.ignore was called with an unwatched path.