[Checkins] [zopefoundation/Acquisition] e27d8d: C extension clean (#16)

GitHub noreply at github.com
Sat Mar 11 14:59:37 CET 2017


  Branch: refs/heads/master
  Home:   https://github.com/zopefoundation/Acquisition
  Commit: e27d8d2ebb922b77c36c7e82349a7676d4d1817e
      https://github.com/zopefoundation/Acquisition/commit/e27d8d2ebb922b77c36c7e82349a7676d4d1817e
  Author: stephan-hof <stephan-hof at users.noreply.github.com>
  Date:   2017-03-11 (Sat, 11 Mar 2017)

  Changed paths:
    M include/ExtensionClass/ExtensionClass.h
    M src/Acquisition/_Acquisition.c

  Log Message:
  -----------
  C extension clean (#16)

* Clean up the __of__method a bit.

* Get rid of temporary variable.

* Use the proper macros in visit and traverse.

* Improve apply_filter.

- No goto
- Handle errors returned by PyObject_IsTrue.
- Use PyObject_CallFunctionObjArgs, which is a more robust way to call functions.
- Ident with 4 spaces.

* Improve some Wrapper methods by

- Replace tabs with spaces.
- Braces on the same line as if.
- indent with 4 spaces.
- Use OBJECT for typecast.

* Refactor CallMethodO.

Did not like they way it handles the ref-count of args.
All the other C-API calls do *not* decref the reference count of args.
So this method behaves not as you usually expect.

Add a new method 'CallMethodArgs' which makes it easier to call methods
where format strings are used to specify positional arguments.

* Let the number protocol wrappers use the new CallMethodArgs method.

This is results in a 20% speedup, since PyObject_CallMethod creates
*always* a new string (unicode in py3) object from the 'char *' pointer.

* Implement the tp_new slot.

With this change it is impossible to create a Wrapper object where
'self->obj' == NULL. __new__ is the very first place where a new
instance is created and there it is ensured that self->obj is set to a
reasonable value. I have seen no other code where self->obj is set.

This allows now to change several places in _Acquisition.c where
self->obj is checked for NULL pointer. Which leads to simpler code.

* Refactor Wrapper_special

* Make use of the fact that 'self->obj' cannot be NULL (if its of type Wrapper)
* Replace tabs with spaces
* Use 4 spaces to indent

* Clean up: Wrapper_findattr_name

- Ident with 4 spaces
- replace tabs with spaces
- More use of braces. I know in some situations they can be omited,
  but they are very dangerous and typically avoided.
- Introduce two macros to check
 - if a string starts with another string
 - if a string is equal to another
- move the swallowing of attribute error into function for reuse.

The string macros and swallow_attribute_error have to potential to be
user for more cleanups.

* Refactor Wrapper_acquire.

* Refactor Wrapper_getattro

Due to __new__ there is always self->obj.

* Refactor Wrapper_[get,set]attro

* Make use of the fact that no empty wrappers exist any more.
* This hand craftet startswith has no meassurable speed boost => omitted.
* The usual spaces/tabs/indent stuff.

* Refactor Wrapper_[rich]compare

* Refactor Wrapper_[repr,str,unicode]

Just the right indentation.

* Refactor: Wrapper_coerce

* Refactor Wrapper_acquire_method

Typical tab/spaces/indent stuff.

* White space tab cleanup.

* Refactor acquire_of.

Instead of two functiond doing nearly the same, create a now one with
the Wrapper class to create as a parameter.
Since only a single argument is needed use METH_O instead of VARARGS.
Now the argument parsing inside the function can be omited, because
python does this for us.

* Refactor [module|capi]_aq_acquire

* Refactor capi_aq_get.

Almost the same as 'aq_acquire'.
Only difference is that the default value is always returned if *any*
excpetion happens.
aq_acquire returns the default value just on Attribute error.

The important bit is to call aq_aquire with
filter=NULL
extra=NULL
explicit=1

* Refactor [capi|module]_aq_base

* __new__ ensures that self->obj is never NULL
* Use METH_O instead of manual parsing a single argument.

* Refactor [capi|module]_aq_parent

* The usual tabs/spaces/indent stuff.
* Use METH_O for a function with a single argument.

* Refactor [module|capi]_aq_self

* Refactor [module|capi]_ac_inner

* The usual tabs/spaces/indent stuff.
* Make use of the fact that __new__ ensures that self->obj is never NULL

* Refactor [module|capi]_aq_chain

* Usual tabs/spaces/indent stuff
* Make use of the fact that __new__ ensures that self->obj is never NULL
* Fix memory leak when PyObject_GetAttr(self, py__parent__) raised
  something else then AttributeError. Before 'result' was leaking.
* The 'err:' section returned 'result' in case of erros. Due to the decref
  before 'result' became an invalid pointer. Returning this pointer
  would have let to a segfault. The proper way to signal an error is by
  returning a NULL pointer. As it is done now.
* Prevent segfault if the return value of
  PyObject_GetAttr(self, py__parent__) had an refcount of 1.
  In that case it Py_DECREF came too early and an access of this pointer
  later would have lead to segfault. Now the reference is kept alive
  until the end of the function. Yes this complicated the other
  assignments to self a bit.

* Refactor [module|capi]_aq_inContextOf

* The usual tab/spaces/indent stuff.
* No checks for self->obj are needed, because __new__ ensures that its always set.
* Handle errors from capi_aq_inner and capi_aq_parent.
  Before the NULL pointer was used as a regular result,
  which would have lead to segfault.
* Don't decref the result from capi_aq_parent too early.
  Before the result of capi_aq_parent() was immediatly Py_DECREF and
  after that *used* ! This was a potential segfault in case the result
  of this had a refcount of 1.
  Now the refcount is decremented at the very end.

* Ident with 4 instead of 2 spaces.

* Get rid of a few typedefs and defines.

Modern verions of python have them in their headers.

* Python has an implementation for ASSIGN, lets use it.

* Use get_base at other places.

New helper function get_inner.

* Use apply__of__ at other places.

* Use STR_EQ in Wrapper_special.

* Add the latest ExtensionClass.h

* Get rid of tabs and some superfluous whitespaces.




More information about the checkins mailing list