Re: svn commit: r1813665 - in /subversion/branches/swig-py3: ./ build/ac-macros/ subversion/bindings/swig/ subversion/bindings/swig/include/ subversion/bindings/swig/python/libsvn_swig_py/

[Daniel Shahaf <d....@daniel.shahaf.name>]

troycurtisjr@apache.org wrote on Sun, Oct 29, 2017 at 03:37:38 -0000:
> On branch swig-py3: Use py3c library in Python swig bindings.
> 
> Add the py3c Python compatibility library and update the python swig bindings
> to use the compatibility functions that it provides. This is the first step to
> getting the swig bindings to support Python 3.
> 
> * ac-macros/py3c.m4:
>   Create a new ac-macro for the py3c library.
> 
> * aclocal.m4:
>   Include the new py3c.m4 file.
> 
> * ac-macros/swig.m4:
>   (SVN_FIND_SWIG): Call the new SVN_PY3C macro to add py3c to the build.
> 
> [in subversion/bindings/swig]
> 
> * include/svn_string.swg
>   (python typemaps: RET_STRING): Replace PyString_FromStringAndSize with
>      PyStr_FromStringAndSize.
>   (python typemaps: (in) svn_stringbuf_t *,
>    (in) const svn_string_t * (svn_string_t value)): 
>      Replace PyString_Check with PyStr_Check and
>      PyString_AS_STRING/PyString_GET_SIZE with PyStr_AsUTF8AndSize.
>   (python typemaps: (out) svn_stringbuf_t *, 
>                     (out) svn_string_t *):
>      Replace PyString_FromStringAndSize with PyStr_FromStringAndSize.
>   (python typemaps: (argout) const char **OUTPUT):
>      Replace PyString_FromString with PyStr_FromString.
> 
> * include/svn_types.swg
>   (python typemaps: (in) (const char *PTR, apr_size_t LEN)): 
>     Replace PyString_Check with PyStr_Check and
>     PyStr_AS_STRING/PyString_GET_SIZE with PyStr_AsUTF8AndSize.
>   (python typemaps: (argout) unsigned char digest[ANY]): 
>     Replace PyString_FromStringAndSize with PyStr_FromStringAndSize.
>   (python typemaps: (in) const unsigned char *digest): 
>     Replace PyString_AsString with PyStr_AsString.
> 
> * include/libsvn_swig_py/swigutil_py.h,
>   include/libsvn_swig_py/swigutil_py.c
>   Include py3c.h to pick up Python 2/3 compatibility defines.
>   (svn_swig_py_as_file): Add new compatibility function.
>   (svn_swig_py_unwrap_string): Add clarifying note to function.
>   (svn_swig_py_svn_exception,
>    svn_swig_py_propinheriteditemarray_to_dict,
>    svn_swig_py_proparray_to_dict,
>    svn_swig_py_locationhash_to_dict,
>    svn_swig_py_c_strings_to_list,
>    svn_swig_py_array_to_list,
>    cstring_to_pystring,
>    convert_string): 
>      Replace PyString_FromString with PyStr_FromString.
>   (make_string_from_ob,
>    make_svn_string_from_ob,
>    svn_swig_py_unwrap_string,
>    ra_callbacks_get_client_string,
>    exeception_to_error):
>      Replace PyString_AsString with PyStr_AsString.
>   (make_string_from_ob, make_svn_string_from_ob):
>      Replace PyString_Check with PyStr_Check.
>   (convert_svn_string_t,
>    svn_swig_py_proparray_to_dict,
>    ra_callbacks_push_or_set_wc_prop):
>      Replace PyString_FromStringAndSize with PyStr_FromStringAndSize.
>   (svn_swig_py_make_file):
>      Replace PyFile_Check/PyFile_AsFile with svn_swig_py_as_file.
>   (read_handler_pyio, ra_callbacks_get_wc_prop):
>      Replace PyString_GET_SIZE/PyString_AS_STRING with PyStr_AsUTF8AndSize.
>   (svn_swig_py_get_commit_log_func):
>      Replace PyString_AS_STRING with PyStr_AsString and PyString_Check with
>      PyStr_Check.
> 
> * core.i
>   (python typemap: 
>    (char *buffer, apr_size_t *len)): 
>      Replace PyString_FromStringAndSize with PyStr_FromStringAndSize.
>   (python typemap: 
>    (const char *data, apr_size_t *len)): 
>      Replace PyString_Check with PyStr_Check, and PyString_AS_STRING/
>      PyString_GET_SIZE with PyStr_AsUTF8AndSize.
>   (python typemap: 
>    const void *value (apr_pool_t *_global_pool, PyObject *_global_py_pool)):
>      Replace PyString_Check with PyStr_Check and PyString_AS_STRING with
>      PyStr_AsString.
>   (python typemap: FILE *): Replace PyFile_AsFile with svn_swig_py_as_file.
> 
> * svn_client.i
>   Include py3c.h to pick up Python 2/3 compatibility defines.
>   (python typemap: (argout) apr_array_header_t **props):
>     Replace PyString_FromStringAndSize with PyStr_FromStringAndSize.
> 
> * svn_delta.i,
>   svn_diff.i,
>   svn_fs.i,
>   svn_ra.i,
>   svn_repos.i,
>   svn_wc.i
>   Include py3c.h to pick up Python 2/3 compatibility defines.
> 
> 
> Added:
>     subversion/branches/swig-py3/build/ac-macros/py3c.m4
> Modified:
>     subversion/branches/swig-py3/aclocal.m4
>     subversion/branches/swig-py3/build/ac-macros/swig.m4
>     subversion/branches/swig-py3/subversion/bindings/swig/core.i
>     subversion/branches/swig-py3/subversion/bindings/swig/include/svn_string.swg
>     subversion/branches/swig-py3/subversion/bindings/swig/include/svn_types.swg
>     subversion/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c
>     subversion/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.h
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_client.i
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_delta.i
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_diff.i
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_fs.i
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_ra.i
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_repos.i
>     subversion/branches/swig-py3/subversion/bindings/swig/svn_wc.i
> 
> +++ subversion/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py/swigutil_py.c Sun Oct 29 03:37:37 2017
> @@ -142,6 +144,30 @@ apr_status_t svn_swig_py_initialize(void
> +FILE *svn_swig_py_as_file(PyObject *pyfile)
> +{
> +#if IS_PY3
> +  FILE *fp = NULL;
> +  int fd = PyFile_AsFileDescriptor(pyfile);
> +  if (fd >= 0)
> +    {
> +      PyObject *mode_obj = PyObject_GetAttrString(pyfile, "mode");
> +      PyObject *mode_byte_obj = PyUnicode_AsUTF8String(mode_obj);
> +      char *mode = PyBytes_AsString(mode_byte_obj);
> +

Shouldn't this code guard for MODE_OBJ potentially being NULL?

Not just on general principles; this function is called in
ra_callbacks_open_tmp_file() on a PyObject returned by a callback written in
user code (in Python).  The PyFile_AsFileDescriptor() doesn't suffice because
it's possible that a user-defined object would (incorrectly?) implement the
file protocol, providing a .fileno() method but no .mode attribute.

PyUnicode_AsUTF8String() can return NULL too.  I didn't check
PyBytes_AsString().

> +      if (mode)
> +        fp = fdopen(fd, mode);
> +
> +      Py_DECREF(mode_obj);
> +      Py_DECREF(mode_byte_obj);
> +    }
> +
> +  return fp;
> +#else
> +  return PyFile_AsFile(pyfile);
> +#endif
> +}

Cheers,

Daniel