respond to review comments; clarify integer return codes

Author: ngoldbaum

stringdtype/stringdtype/src/dtype.c

@@ -124,13 +124,19 @@ stringdtype_setitem(StringDTypeObject *NPY_UNUSED(descr), PyObject *obj,
     ssfree((ss *)dataptr);
 
     // copies contents of val into item_val->buf
-    if (ssnewlen(val, length, (ss *)dataptr) == -1) {
-        PyErr_SetString(PyExc_MemoryError, "ssnewlen failed");
+    int res = ssnewlen(val, length, (ss *)dataptr);
+    // val_obj must stay alive until here to ensure *val* doesn't get
+    // deallocated
+    Py_DECREF(val_obj);
+    if (res == -1) {
+        PyErr_NoMemory();
         return -1;
     }
+    else if (res == -2) {
+        // this should never happen
+        assert(0);
+    }
 
-    // val_obj must stay alive until here to ensure *val* doesn't get
-    // deallocated
     Py_DECREF(val_obj);
     return 0;
 }
@@ -156,10 +162,6 @@ stringdtype_getitem(StringDTypeObject *NPY_UNUSED(descr), char **dataptr)
     PyObject *res = PyObject_CallFunctionObjArgs((PyObject *)StringScalar_Type,
                                                  val_obj, NULL);
 
-    if (res == NULL) {
-        return NULL;
-    }
-
     Py_DECREF(val_obj);
 
     return res;

stringdtype/stringdtype/src/static_string.c

@@ -4,7 +4,7 @@ int
 ssnewlen(const char *init, size_t len, ss *to_init)
 {
     if ((to_init->buf != NULL) || (to_init->len != 0)) {
-        return -1;
+        return -2;
     }
 
     // one extra byte for null terminator
@@ -47,7 +47,7 @@ int
 ssnewemptylen(size_t num_bytes, ss *out)
 {
     if (out->len != 0 || out->buf != NULL) {
-        return -1;
+        return -2;
     }
 
     char *buf = (char *)malloc(sizeof(char) * (num_bytes + 1));

stringdtype/stringdtype/src/static_string.h

@@ -10,26 +10,27 @@ typedef struct ss {
 } ss;
 
 // Allocates a new buffer for *to_init*, filling with the copied contents of
-// *init* and sets *to_init->len* to *len*.
+// *init* and sets *to_init->len* to *len*. Returns -1 if malloc fails and -2
+// if *to_init* is not empty. Returns 0 on success.
 int
 ssnewlen(const char *init, size_t len, ss *to_init);
 
-// Frees the internal string buffer held by *str*. If str->buf is NULL, does
-// nothing.
+// Sets len to 0 and if str->buf is not already NULL, frees it and sets it to
+// NULL. Cannot fail.
 void
 ssfree(ss *str);
 
 // copies the contents out *in* into *out*. Allocates a new string buffer for
-// *out*, checks that *out->buf* is NULL and *out->len* is zero and errors
-// otherwise. Also errors if malloc fails.
+// *out*. Returns -1 if malloc fails and -2 if *out* is not empty. Returns 0 on
+// success.
 int
 ssdup(ss *in, ss *out);
 
 // Allocates a new string buffer for *out* with enough capacity to store
 // *num_bytes* of text. The actual allocation is num_bytes + 1 bytes, to
 // account for the null terminator. Does not do any initialization, the caller
-// must initialize and null-terminate the string buffer. Errors if *out*
-// already contains data or if malloc fails.
+// must initialize and null-terminate the string buffer. Returns -1 if malloc
+// fails and -2 if *out* is not empty. Returns 0 on success.
 int
 ssnewemptylen(size_t num_bytes, ss *out);