Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

MNT: constant string arrays instead of pointers in C #28985

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
May 23, 2025
+40 −35
Merged

MNT: constant string arrays instead of pointers in C #28985

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
db5535a
MNT: constant string arrays instead of pointers in C
DimitriPapadopoulos May 16, 2025
a5401fd
MNT: alternative handling of trailing null character
DimitriPapadopoulos May 16, 2025
22132f1
MNT: more const'ness for arrays of string literals in C
DimitriPapadopoulos May 16, 2025
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
4 changes: 2 additions & 2 deletions 4 numpy/_core/src/common/npy_cpu_features.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ npy__cpu_validate_baseline(void)
static int
npy__cpu_check_env(int disable, const char *env) {

static const char *names[] = {
static const char *const names[] = {
"enable", "disable",
"NPY_ENABLE_CPU_FEATURES", "NPY_DISABLE_CPU_FEATURES",
"During parsing environment variable: 'NPY_ENABLE_CPU_FEATURES':\n",
Expand Down Expand Up @@ -277,7 +277,7 @@ npy__cpu_check_env(int disable, const char *env) {
char *notsupp_cur = &notsupp[0];

//comma and space including (htab, vtab, CR, LF, FF)
const char *delim = ", \t\v\r\n\f";
const char delim[] = ", \t\v\r\n\f";
char *feature = strtok(features, delim);
while (feature) {
if (npy__cpu_baseline_fid(feature) > 0){
Expand Down
6 changes: 3 additions & 3 deletions 6 numpy/_core/src/multiarray/compiled_base.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,11 +920,11 @@ arr_interp_complex(PyObject *NPY_UNUSED(self), PyObject *const *args, Py_ssize_t
return NULL;
}

static const char *EMPTY_SEQUENCE_ERR_MSG = "indices must be integral: the provided " \
static const char EMPTY_SEQUENCE_ERR_MSG[] = "indices must be integral: the provided " \
"empty sequence was inferred as float. Wrap it with " \
"'np.array(indices, dtype=np.intp)'";

static const char *NON_INTEGRAL_ERROR_MSG = "only int indices permitted";
static const char NON_INTEGRAL_ERROR_MSG[] = "only int indices permitted";

/* Convert obj to an ndarray with integer dtype or fail */
static PyArrayObject *
Expand Down Expand Up @@ -1465,7 +1465,7 @@ arr_add_docstring(PyObject *NPY_UNUSED(dummy), PyObject *const *args, Py_ssize_t
PyObject *obj;
PyObject *str;
const char *docstr;
static char *msg = "already has a different docstring";
static const char msg[] = "already has a different docstring";

/* Don't add docstrings */
#if PY_VERSION_HEX > 0x030b0000
Expand Down
39 changes: 22 additions & 17 deletions 39 numpy/_core/src/multiarray/stringdtype/casts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ load_non_nullable_string(char *in, int has_null, const npy_static_string *defaul
const npy_packed_static_string *ps = (npy_packed_static_string *)in;
int isnull = NpyString_load(allocator, ps, string_to_load);
if (isnull == -1) {
const char *msg = "Failed to load string for conversion to a non-nullable type";
const char msg[] = "Failed to load string for conversion to a non-nullable type";
if (has_gil)
{
PyErr_SetString(PyExc_MemoryError, msg);
Expand All @@ -617,7 +617,7 @@ load_non_nullable_string(char *in, int has_null, const npy_static_string *defaul
}
else if (isnull) {
if (has_null) {
const char *msg = "Arrays with missing data cannot be converted to a non-nullable type";
const char msg[] = "Arrays with missing data cannot be converted to a non-nullable type";
if (has_gil)
{
PyErr_SetString(PyExc_ValueError, msg);
Expand Down Expand Up @@ -821,8 +821,8 @@ static PyType_Slot s2int_slots[] = {

static const char *
make_s2type_name(NPY_TYPES typenum) {
const char *prefix = "cast_StringDType_to_";
size_t plen = strlen(prefix);
const char prefix[] = "cast_StringDType_to_";
size_t plen = sizeof(prefix)/sizeof(char) - 1;

const char *type_name = typenum_to_cstr(typenum);
size_t nlen = strlen(type_name);
Expand All @@ -833,31 +833,36 @@ make_s2type_name(NPY_TYPES typenum) {
return NULL;
}

// memcpy instead of strcpy to avoid stringop-truncation warning, since
// we are not including the trailing null character
memcpy(buf, prefix, plen);
strncat(buf, type_name, nlen);
// memcpy instead of strcpy/strncat to avoid stringop-truncation warning,
// since we are not including the trailing null character
char *p = buf;
memcpy(p, prefix, plen);
p += plen;
memcpy(p, type_name, nlen);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Copy link
Contributor Author

@DimitriPapadopoulos DimitriPapadopoulos May 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment in the relevant commit:

Trying to avoid this compiler warning:

 ../numpy/_core/src/multiarray/stringdtype/casts.cpp:860:12: warning: 'char* strncat(char*, const char*, size_t)' specified bound 15 equals source length [-Wstringop-overflow=]
  860 |     strncat(buf, suffix, slen);
      |     ~~~~~~~^~~~~~~~~~~~~~~~~~~

The previous code had already been using memcpy() instead of strncpy() to avoid a similar warning. The new code extends the logic to strncat(). In any case, the code should be consistent, and use only one set of methods:

  • Use strcpy()/strcat() all along, relying on the trailing NUL to find the characters to copy. This would avoid compiler warnings. I know these functions feel less secure, but in this case they are not since we know for sure the destination buf is large enough.
  • Use strncpy()/strncat() all along, looking for a trailing NUL while copying, but omitting writing the trailing NUL in the destinationbuf, since it is already initialised by PyMem_RawCalloc(). This results in the above compiler warning, because the trailing NUL is omitted.
  • Use memcpy() all along, relying on the already calculated length of the strings. That should be the most efficient option and avoids compiler warnings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good 👍

return buf;
}

static const char *
make_type2s_name(NPY_TYPES typenum) {
const char *prefix = "cast_";
size_t plen = strlen(prefix);
const char prefix[] = "cast_";
size_t plen = sizeof(prefix)/sizeof(char) - 1;

const char *type_name = typenum_to_cstr(typenum);
size_t nlen = strlen(type_name);

const char *suffix = "_to_StringDType";
size_t slen = strlen(suffix);
const char suffix[] = "_to_StringDType";
size_t slen = sizeof(prefix)/sizeof(char) - 1;

char *buf = (char *)PyMem_RawCalloc(sizeof(char), plen + nlen + slen + 1);

// memcpy instead of strcpy to avoid stringop-truncation warning, since
// we are not including the trailing null character
memcpy(buf, prefix, plen);
strncat(buf, type_name, nlen);
strncat(buf, suffix, slen);
// memcpy instead of strcpy/strncat to avoid stringop-truncation warning,
// since we are not including the trailing null character
char *p = buf;
memcpy(p, prefix, plen);
p += plen;
memcpy(p, type_name, nlen);
p += nlen;
memcpy(p, suffix, slen);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this too

return buf;
}

Expand Down
2 changes: 1 addition & 1 deletion 2 numpy/_core/src/multiarray/stringdtype/static_string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ NpyString_release_allocators(size_t length, npy_string_allocator *allocators[])
}
}

static const char * const EMPTY_STRING = "";
static const char EMPTY_STRING[] = "";

/*NUMPY_API
* Extract the packed contents of *packed_string* into *unpacked_string*.
Expand Down
12 changes: 6 additions & 6 deletions 12 numpy/_core/src/umath/string_ufuncs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1521,7 +1521,7 @@ init_string_ufuncs(PyObject *umath)
dtypes[0] = NPY_OBJECT;
dtypes[1] = NPY_BOOL;

const char *unary_buffer_method_names[] = {
const char *const unary_buffer_method_names[] = {
"isalpha", "isalnum", "isdigit", "isspace", "islower",
"isupper", "istitle", "isdecimal", "isnumeric",
};
Expand Down Expand Up @@ -1635,7 +1635,7 @@ init_string_ufuncs(PyObject *umath)
dtypes[2] = dtypes[3] = NPY_INT64;
dtypes[4] = NPY_BOOL;

const char *startswith_endswith_names[] = {
const char *const startswith_endswith_names[] = {
"startswith", "endswith"
};

Expand Down Expand Up @@ -1664,7 +1664,7 @@ init_string_ufuncs(PyObject *umath)

dtypes[0] = dtypes[1] = NPY_OBJECT;

const char *strip_whitespace_names[] = {
const char *const strip_whitespace_names[] = {
"_lstrip_whitespace", "_rstrip_whitespace", "_strip_whitespace"
};

Expand All @@ -1691,7 +1691,7 @@ init_string_ufuncs(PyObject *umath)

dtypes[0] = dtypes[1] = dtypes[2] = NPY_OBJECT;

const char *strip_chars_names[] = {
const char *const strip_chars_names[] = {
"_lstrip_chars", "_rstrip_chars", "_strip_chars"
};

Expand Down Expand Up @@ -1750,7 +1750,7 @@ init_string_ufuncs(PyObject *umath)

dtypes[1] = NPY_INT64;

const char *center_ljust_rjust_names[] = {
const char *const center_ljust_rjust_names[] = {
"_center", "_ljust", "_rjust"
};

Expand Down Expand Up @@ -1827,7 +1827,7 @@ init_string_ufuncs(PyObject *umath)
dtypes[0] = dtypes[1] = dtypes[3] = dtypes[4] = dtypes[5] = NPY_OBJECT;
dtypes[2] = NPY_INT64;

const char *partition_names[] = {"_partition_index", "_rpartition_index"};
const char *const partition_names[] = {"_partition_index", "_rpartition_index"};

static STARTPOSITION partition_startpositions[] = {
STARTPOSITION::FRONT, STARTPOSITION::BACK
Expand Down
10 changes: 5 additions & 5 deletions 10 numpy/_core/src/umath/stringdtype_ufuncs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2605,7 +2605,7 @@ add_object_and_unicode_promoters(PyObject *umath, const char* ufunc_name,
NPY_NO_EXPORT int
init_stringdtype_ufuncs(PyObject *umath)
{
static const char *comparison_ufunc_names[6] = {
static const char *const comparison_ufunc_names[6] = {
"equal", "not_equal",
"less", "less_equal", "greater_equal", "greater",
};
Expand Down Expand Up @@ -2654,7 +2654,7 @@ init_stringdtype_ufuncs(PyObject *umath)
return -1;
}

const char *unary_loop_names[] = {
const char *const unary_loop_names[] = {
"isalpha", "isdecimal", "isdigit", "isnumeric", "isspace",
"isalnum", "istitle", "isupper", "islower",
};
Expand Down Expand Up @@ -2874,7 +2874,7 @@ init_stringdtype_ufuncs(PyObject *umath)
&PyArray_StringDType, &PyArray_StringDType
};

const char *strip_whitespace_names[] = {
const char *const strip_whitespace_names[] = {
"_lstrip_whitespace", "_rstrip_whitespace", "_strip_whitespace",
};

Expand All @@ -2898,7 +2898,7 @@ init_stringdtype_ufuncs(PyObject *umath)
&PyArray_StringDType, &PyArray_StringDType, &PyArray_StringDType
};

const char *strip_chars_names[] = {
const char *const strip_chars_names[] = {
"_lstrip_chars", "_rstrip_chars", "_strip_chars",
};

Expand Down Expand Up @@ -3082,7 +3082,7 @@ init_stringdtype_ufuncs(PyObject *umath)
&PyArray_StringDType
};

const char *partition_names[] = {"_partition", "_rpartition"};
const char *const partition_names[] = {"_partition", "_rpartition"};

static STARTPOSITION partition_startpositions[] = {
STARTPOSITION::FRONT, STARTPOSITION::BACK
Expand Down
2 changes: 1 addition & 1 deletion 2 numpy/f2py/rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1154,7 +1154,7 @@
'frompyobj': [
' #setdims#;',
' capi_#varname#_intent |= #intent#;',
(' const char * capi_errmess = "#modulename#.#pyname#:'
(' const char capi_errmess[] = "#modulename#.#pyname#:'
' failed to create array from the #nth# `#varname#`";'),
{isintent_hide:
' capi_#varname#_as_array = ndarray_from_pyobj('
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.