Skip to content

BUG: revise string_from_pyobj/try_pyarr_from_string with respect to malloc and copy (the second round) #19251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 17, 2021
Merged

BUG: revise string_from_pyobj/try_pyarr_from_string with respect to malloc and copy (the second round) #19251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
abe3eb2
BUG: revise string_from_pyobj/try_pyarr_from_string with respect to m…
pearu Jun 15, 2021
fd51a8e
Fix lint
pearu Jun 15, 2021
3ca331b
Fix lint - 2
pearu Jun 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 128 additions & 52 deletions numpy/f2py/cfuncs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@
"""
cppmacros['STRINGMALLOC'] = """\
#define STRINGMALLOC(str,len)\\
if ((str = (string)malloc(sizeof(char)*(len+1))) == NULL) {\\
if ((str = (string)malloc(len+1)) == NULL) {\\
PyErr_SetString(PyExc_MemoryError, \"out of memory\");\\
goto capi_fail;\\
} else {\\
Expand All @@ -479,20 +479,41 @@
cppmacros['STRINGFREE'] = """\
#define STRINGFREE(str) do {if (!(str == NULL)) free(str);} while (0)
"""
needs['STRINGPADN'] = ['string.h']
cppmacros['STRINGPADN'] = """\
/*
STRINGPADN replaces null values with padding values from the right.

`to` must have size of at least N bytes.

If the `to[N-1]` has null value, then replace it and all the
preceeding nulls with the given padding.

STRINGPADN(to, N, PADDING, NULLVALUE) is an inverse operation.
*/
#define STRINGPADN(to, N, NULLVALUE, PADDING) \\
do { \\
int _m = (N); \\
char *_to = (to); \\
for (_m -= 1; _m >= 0 && _to[_m] == NULLVALUE; _m--) { \\
_to[_m] = PADDING; \\
} \\
} while (0)
"""
needs['STRINGCOPYN'] = ['string.h', 'FAILNULL']
cppmacros['STRINGCOPYN'] = """\
#define STRINGCOPYN(to,from,buf_size) \\
/*
STRINGCOPYN copies N bytes.

`to` and `from` buffers must have sizes of at least N bytes.
*/
#define STRINGCOPYN(to,from,N) \\
do { \\
int _m = (buf_size); \\
int _m = (N); \\
char *_to = (to); \\
char *_from = (from); \\
FAILNULL(_to); FAILNULL(_from); \\
(void)strncpy(_to, _from, sizeof(char)*_m); \\
_to[_m-1] = '\\0'; \\
/* Padding with spaces instead of nulls */ \\
for (_m -= 2; _m >= 0 && _to[_m] == '\\0'; _m--) { \\
_to[_m] = ' '; \\
} \\
(void)strncpy(_to, _from, _m); \\
} while (0)
"""
needs['STRINGCOPY'] = ['string.h', 'FAILNULL']
Expand Down Expand Up @@ -623,71 +644,127 @@
}"""
needs['try_pyarr_from_string'] = ['STRINGCOPYN', 'PRINTPYOBJERR', 'string']
cfuncs['try_pyarr_from_string'] = """\
static int try_pyarr_from_string(PyObject *obj,const string str) {
PyArrayObject *arr = NULL;
if (PyArray_Check(obj) && (!((arr = (PyArrayObject *)obj) == NULL)))
{ STRINGCOPYN(PyArray_DATA(arr),str,PyArray_NBYTES(arr)); }
return 1;
/*
try_pyarr_from_string copies str[:len(obj)] to the data of an `ndarray`.

If obj is an `ndarray`, it is assumed to be contiguous.

If the specified len==-1, str must be null-terminated.
*/
static int try_pyarr_from_string(PyObject *obj,
const string str, const int len) {
#ifdef DEBUGCFUNCS
fprintf(stderr, "try_pyarr_from_string(str='%s', len=%d, obj=%p)\\n",
(char*)str,len, obj);
#endif
if (PyArray_Check(obj)) {
PyArrayObject *arr = (PyArrayObject *)obj;
assert(ISCONTIGUOUS(arr));
string buf = PyArray_DATA(arr);
npy_intp n = len;
if (n == -1) {
/* Assuming null-terminated str. */
n = strlen(str);
}
if (n > PyArray_NBYTES(arr)) {
n = PyArray_NBYTES(arr);
}
STRINGCOPYN(buf, str, n);
return 1;
}
capi_fail:
PRINTPYOBJERR(obj);
PyErr_SetString(#modulename#_error,\"try_pyarr_from_string failed\");
PyErr_SetString(#modulename#_error, \"try_pyarr_from_string failed\");
return 0;
}
"""
needs['string_from_pyobj'] = ['string', 'STRINGMALLOC', 'STRINGCOPYN']
cfuncs['string_from_pyobj'] = """\
/*
Create a new string buffer `str` of at most length `len` from a
Python string-like object `obj`.

The string buffer has given size (len) or the size of inistr when len==-1.

The string buffer is padded with blanks: in Fortran, trailing blanks
are insignificant contrary to C nulls.
*/
static int
string_from_pyobj(string *str,int *len,const string inistr,PyObject *obj,const char *errmess)
string_from_pyobj(string *str, int *len, const string inistr, PyObject *obj,
const char *errmess)
{
PyArrayObject *arr = NULL;
PyObject *tmp = NULL;
string buf = NULL;
npy_intp n = -1;
#ifdef DEBUGCFUNCS
fprintf(stderr,\"string_from_pyobj(str='%s',len=%d,inistr='%s',obj=%p)\\n\",(char*)str,*len,(char *)inistr,obj);
fprintf(stderr,\"string_from_pyobj(str='%s',len=%d,inistr='%s',obj=%p)\\n\",
(char*)str, *len, (char *)inistr, obj);
#endif
if (obj == Py_None) {
if (*len == -1)
*len = strlen(inistr); /* Will this cause problems? */
STRINGMALLOC(*str,*len);
STRINGCOPYN(*str,inistr,*len+1);
return 1;
n = strlen(inistr);
buf = inistr;
}
if (PyArray_Check(obj)) {
if ((arr = (PyArrayObject *)obj) == NULL)
goto capi_fail;
else if (PyArray_Check(obj)) {
PyArrayObject *arr = (PyArrayObject *)obj;
if (!ISCONTIGUOUS(arr)) {
PyErr_SetString(PyExc_ValueError,\"array object is non-contiguous.\");
PyErr_SetString(PyExc_ValueError,
\"array object is non-contiguous.\");
goto capi_fail;
}
if (*len == -1)
*len = (PyArray_ITEMSIZE(arr))*PyArray_SIZE(arr);
STRINGMALLOC(*str,*len);
STRINGCOPYN(*str,PyArray_DATA(arr),*len+1);
return 1;
}
if (PyBytes_Check(obj)) {
tmp = obj;
Py_INCREF(tmp);
}
else if (PyUnicode_Check(obj)) {
tmp = PyUnicode_AsASCIIString(obj);
n = PyArray_NBYTES(arr);
buf = PyArray_DATA(arr);
n = strnlen(buf, n);
}
else {
PyObject *tmp2;
tmp2 = PyObject_Str(obj);
if (tmp2) {
tmp = PyUnicode_AsASCIIString(tmp2);
Py_DECREF(tmp2);
if (PyBytes_Check(obj)) {
tmp = obj;
Py_INCREF(tmp);
}
else if (PyUnicode_Check(obj)) {
tmp = PyUnicode_AsASCIIString(obj);
}
else {
tmp = NULL;
PyObject *tmp2;
tmp2 = PyObject_Str(obj);
if (tmp2) {
tmp = PyUnicode_AsASCIIString(tmp2);
Py_DECREF(tmp2);
}
else {
tmp = NULL;
}
}
if (tmp == NULL) goto capi_fail;
n = PyBytes_GET_SIZE(tmp);
buf = PyBytes_AS_STRING(tmp);
}
if (*len == -1) {
/* TODO: change the type of `len` so that we can remove this */
if (n > NPY_MAX_INT) {
PyErr_SetString(PyExc_OverflowError,
"object too large for a 32-bit int");
goto capi_fail;
}
*len = n;
}
if (tmp == NULL) goto capi_fail;
if (*len == -1)
*len = PyBytes_GET_SIZE(tmp);
STRINGMALLOC(*str,*len);
STRINGCOPYN(*str,PyBytes_AS_STRING(tmp),*len+1);
Py_DECREF(tmp);
else if (*len < n) {
/* discard the last (len-n) bytes of input buf */
n = *len;
}
if (n < 0 || *len < 0 || buf == NULL) {
goto capi_fail;
}
STRINGMALLOC(*str, *len); // *str is allocated with size (*len + 1)
if (n < *len) {
/*
Pad fixed-width string with nulls. The caller will replace
nulls with blanks when the corresponding argument is not
intent(c).
*/
memset(*str + n, '\\0', *len - n);
}
STRINGCOPYN(*str, buf, n);
Py_XDECREF(tmp);
return 1;
capi_fail:
Py_XDECREF(tmp);
Expand All @@ -702,7 +779,6 @@
}
"""


needs['char_from_pyobj'] = ['int_from_pyobj']
cfuncs['char_from_pyobj'] = """\
static int
Expand Down
42 changes: 32 additions & 10 deletions numpy/f2py/rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,8 @@
'\tint #name#_return_value_len = 0;'],
'callfortran':'#name#_return_value,#name#_return_value_len,',
'callfortranroutine':['\t#name#_return_value_len = #rlength#;',
'\tif ((#name#_return_value = (string)malloc(sizeof(char)*(#name#_return_value_len+1))) == NULL) {',
'\tif ((#name#_return_value = (string)malloc('
'#name#_return_value_len+1) == NULL) {',
'\t\tPyErr_SetString(PyExc_MemoryError, \"out of memory\");',
'\t\tf2py_success = 0;',
'\t} else {',
Expand Down Expand Up @@ -942,31 +943,52 @@
'\tPyObject *#varname#_capi = Py_None;'],
'callfortran':'#varname#,',
'callfortranappend':'slen(#varname#),',
'pyobjfrom':{debugcapi: '\tfprintf(stderr,"#vardebugshowvalue#\\n",slen(#varname#),#varname#);'},
'pyobjfrom':[
{debugcapi:
'\tfprintf(stderr,'
'"#vardebugshowvalue#\\n",slen(#varname#),#varname#);'},
# The trailing null value for Fortran is blank.
{l_and(isintent_out, l_not(isintent_c)):
"\t\tSTRINGPADN(#varname#, slen(#varname#), ' ', '\\0');"},
],
'return': {isintent_out: ',#varname#'},
'need': ['len..'], # 'STRINGFREE'],
'need': ['len..',
{l_and(isintent_out, l_not(isintent_c)): 'STRINGPADN'}],
'_check':isstring
}, { # Common
'frompyobj': """\
'frompyobj': [
"""\
\tslen(#varname#) = #length#;
\tf2py_success = #ctype#_from_pyobj(&#varname#,&slen(#varname#),#init#,#varname#_capi,\"#ctype#_from_pyobj failed in converting #nth# `#varname#\' of #pyname# to C #ctype#\");
\tf2py_success = #ctype#_from_pyobj(&#varname#,&slen(#varname#),#init#,"""
"""#varname#_capi,\"#ctype#_from_pyobj failed in converting #nth#"""
"""`#varname#\' of #pyname# to C #ctype#\");
\tif (f2py_success) {""",
# The trailing null value for Fortran is blank.
{l_not(isintent_c):
"\t\tSTRINGPADN(#varname#, slen(#varname#), '\\0', ' ');"},
],
'cleanupfrompyobj': """\
\t\tSTRINGFREE(#varname#);
\t} /*if (f2py_success) of #varname#*/""",
'need': ['#ctype#_from_pyobj', 'len..', 'STRINGFREE'],
'need': ['#ctype#_from_pyobj', 'len..', 'STRINGFREE',
{l_not(isintent_c): 'STRINGPADN'}],
'_check':isstring,
'_depend':''
}, { # Not hidden
'argformat': {isrequired: 'O'},
'keyformat': {isoptional: 'O'},
'args_capi': {isrequired: ',&#varname#_capi'},
'keys_capi': {isoptional: ',&#varname#_capi'},
'pyobjfrom': {isintent_inout: '''\
\tf2py_success = try_pyarr_from_#ctype#(#varname#_capi,#varname#);
\tif (f2py_success) {'''},
'pyobjfrom': [
{l_and(isintent_inout, l_not(isintent_c)):
"\t\tSTRINGPADN(#varname#, slen(#varname#), ' ', '\\0');"},
{isintent_inout: '''\
\tf2py_success = try_pyarr_from_#ctype#(#varname#_capi, #varname#,
\t slen(#varname#));
\tif (f2py_success) {'''}],
'closepyobjfrom': {isintent_inout: '\t} /*if (f2py_success) of #varname# pyobjfrom*/'},
'need': {isintent_inout: 'try_pyarr_from_#ctype#'},
'need': {isintent_inout: 'try_pyarr_from_#ctype#',
l_and(isintent_inout, l_not(isintent_c)): 'STRINGPADN'},
'_check': l_and(isstring, isintent_nothide)
}, { # Hidden
'_check': l_and(isstring, isintent_hide)
Expand Down
2 changes: 1 addition & 1 deletion numpy/f2py/tests/src/array_from_pyobj/wrapmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ static PyObject *f2py_rout_wrap_attrs(PyObject *capi_self,
PyObject *strides = NULL;
char s[100];
int i;
memset(s,0,100*sizeof(char));
memset(s,0,100);
if (!PyArg_ParseTuple(capi_args,"O!|:wrap.attrs",
&PyArray_Type,&arr_capi))
return NULL;
Expand Down
6 changes: 3 additions & 3 deletions numpy/f2py/tests/test_return_character.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ def check_function(self, t, tname):
#assert_(_raises(ValueError, t, array([77,87])))
#assert_(_raises(ValueError, t, array(77)))
elif tname in ['ts', 'ss']:
assert_(t(23) == b'23 ', repr(t(23)))
assert_(t(23) == b'23', repr(t(23)))
assert_(t('123456789abcdef') == b'123456789a')
elif tname in ['t5', 's5']:
assert_(t(23) == b'23 ', repr(t(23)))
assert_(t('ab') == b'ab ', repr(t('ab')))
assert_(t(23) == b'23', repr(t(23)))
assert_(t('ab') == b'ab', repr(t('ab')))
assert_(t('123456789abcdef') == b'12345')
else:
raise NotImplementedError
Expand Down
Loading