[PATCH 0/8] Move py C utility code to pyutil; guard overflow

[PATCH 0/8] Move py C utility code to pyutil; guard overflow

[Rob Browning]

Proposed for main.

This starts moving some of the utility functions to src/bup/pyutil.*,
so that we can use them everywhere, e.g. hashsplit, bupsplit...

Rob Browning (8):
  bup_shared_cflags: add -Winline
  Create src/pyutil.c for utility functions
  pyutil: add INTEGER_TO_PY as BUP_LONGISH_TO_PY
  pyutil: add bup_uint_from_py bup_ulong_from_py bup_ullong_from_py
  pyutil: add BUP_ASSIGN_PYLONG_TO_INTEGRAL; use EXPR_SIGNED
  _helpers: remove vestigial py2 utimes related code
  HashSplitter_init: guard against bits/fanbits overflow
  Reject bup_getpwuid bup_getgrgid argument overflow

 GNUmakefile          |   4 +-
 lib/bup/_hashsplit.c |  14 +-
 lib/bup/_helpers.c   | 368 +++++--------------------------------------
 src/bup/pyutil.c     |  93 +++++++++++
 src/bup/pyutil.h     |  54 +++++++
 5 files changed, 196 insertions(+), 337 deletions(-)
 create mode 100644 src/bup/pyutil.c
 create mode 100644 src/bup/pyutil.h

-- 
2.39.2

[PATCH 1/8] bup_shared_cflags: add -Winline

[Rob Browning]

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 GNUmakefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/GNUmakefile b/GNUmakefile
index 6300acf6c..d762f32b7 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -43,7 +43,7 @@ os := $(call shout,$(os),Unable to determine OS)
 DESTDIR ?=
 TARGET_ARCH ?=
 
-bup_shared_cflags := -O2 -Wall -Werror -Wformat=2 -MMD -MP
+bup_shared_cflags := -O2 -Wall -Werror -Wformat=2 -Winline -MMD -MP
 bup_shared_cflags := -Wno-unknown-pragmas -Wsign-compare $(bup_shared_cflags)
 bup_shared_cflags := -D_FILE_OFFSET_BITS=64 $(bup_shared_cflags)
 bup_shared_cflags := $(bup_config_cflags) $(bup_shared_cflags)
-- 
2.39.2

[PATCH 2/8] Create src/pyutil.c for utility functions

[Rob Browning]

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
---
 GNUmakefile        |  2 +-
 lib/bup/_helpers.c | 29 ++---------------------------
 src/bup/pyutil.c   | 37 +++++++++++++++++++++++++++++++++++++
 src/bup/pyutil.h   |  4 ++++
 4 files changed, 44 insertions(+), 28 deletions(-)
 create mode 100644 src/bup/pyutil.c
 create mode 100644 src/bup/pyutil.h

diff --git a/GNUmakefile b/GNUmakefile
index d762f32b7..d218698df 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -197,7 +197,7 @@ lib/cmd/bup: lib/cmd/bup.c src/bup/compat.c src/bup/io.c
 
 clean_paths += lib/bup/_helpers$(soext)
 generated_dependencies += lib/bup/_helpers.d
-lib/bup/_helpers$(soext): lib/bup/_helpers.c lib/bup/bupsplit.c lib/bup/_hashsplit.c
+lib/bup/_helpers$(soext): lib/bup/_helpers.c src/bup/pyutil.c lib/bup/bupsplit.c lib/bup/_hashsplit.c
 	$(CC) $(helpers_cflags) $(CPPFLAGS) $(CFLAGS) $^ \
 	  $(helpers_ldflags) $(LDFLAGS) -o $@
 
diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 507b5842e..6874a2bc6 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -64,8 +64,9 @@
 # pragma GCC diagnostic pop
 #endif
 
-#include "bupsplit.h"
 #include "bup/intprops.h"
+#include "bup/pyutil.h"
+#include "bupsplit.h"
 #include "_hashsplit.h"
 
 #if defined(FS_IOC_GETFLAGS) && defined(FS_IOC_SETFLAGS)
@@ -108,32 +109,6 @@ typedef struct {
 #define rbuf_argf "y#"
 #define wbuf_argf "y*"
 
-
-static void *checked_calloc(size_t n, size_t size)
-{
-    void *result = calloc(n, size);
-    if (!result)
-        PyErr_NoMemory();
-    return result;
-}
-
-static void *checked_malloc(size_t n, size_t size)
-{
-    size_t total;
-    if (!INT_MULTIPLY_OK(n, size, &total))
-    {
-        PyErr_Format(PyExc_OverflowError,
-                     "request to allocate %zu items of size %zu is too large",
-                     n, size);
-        return NULL;
-    }
-    void *result = malloc(total);
-    if (!result)
-        return PyErr_NoMemory();
-    return result;
-}
-
-
 #ifndef htonll
 // This function should technically be macro'd out if it's going to be used
 // more than ocasionally.  As of this writing, it'll actually never be called
diff --git a/src/bup/pyutil.c b/src/bup/pyutil.c
new file mode 100644
index 000000000..53ca39c1d
--- /dev/null
+++ b/src/bup/pyutil.c
@@ -0,0 +1,37 @@
+#define _LARGEFILE64_SOURCE 1
+#define PY_SSIZE_T_CLEAN 1
+#undef NDEBUG
+#include "../../config/config.h"
+
+// According to Python, its header has to go first:
+//   http://docs.python.org/3/c-api/intro.html#include-files
+#include <Python.h>
+
+#include "bup/pyutil.h"
+
+#include "bup/intprops.h"
+
+
+void *checked_calloc(size_t n, size_t size)
+{
+    void *result = calloc(n, size);
+    if (!result)
+        PyErr_NoMemory();
+    return result;
+}
+
+void *checked_malloc(size_t n, size_t size)
+{
+    size_t total;
+    if (!INT_MULTIPLY_OK(n, size, &total))
+    {
+        PyErr_Format(PyExc_OverflowError,
+                     "request to allocate %zu items of size %zu is too large",
+                     n, size);
+        return NULL;
+    }
+    void *result = malloc(total);
+    if (!result)
+        return PyErr_NoMemory();
+    return result;
+}
diff --git a/src/bup/pyutil.h b/src/bup/pyutil.h
new file mode 100644
index 000000000..680ca5460
--- /dev/null
+++ b/src/bup/pyutil.h
@@ -0,0 +1,4 @@
+#pragma once
+
+void *checked_calloc(size_t n, size_t size);
+void *checked_malloc(size_t n, size_t size);
-- 
2.39.2

[PATCH 3/8] pyutil: add INTEGER_TO_PY as BUP_LONGISH_TO_PY

[Rob Browning]

Change to longish to hint that it only handles up to
long (long)s.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/_helpers.c | 47 ++++++++++++++++++++++------------------------
 src/bup/pyutil.h   |  7 +++++++
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 6874a2bc6..d58cf9954 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -125,9 +125,6 @@ static uint64_t htonll(uint64_t value)
 
 #define INTEGRAL_ASSIGNMENT_FITS(dest, src) INT_ADD_OK(src, 0, dest)
 
-#define INTEGER_TO_PY(x) \
-    EXPR_SIGNED(x) ? PyLong_FromLongLong(x) : PyLong_FromUnsignedLongLong(x)
-
 
 static int bup_ulong_from_py(unsigned long *x, PyObject *py, const char *name)
 {
@@ -1359,22 +1356,22 @@ static PyObject *stat_struct_to_py(const struct stat *st,
 {
     // We can check the known (via POSIX) signed and unsigned types at
     // compile time, but not (easily) the unspecified types, so handle
-    // those via INTEGER_TO_PY().  Assumes ns values will fit in a
+    // those via BUP_LONGISH_TO_PY().  Assumes ns values will fit in a
     // long.
     return Py_BuildValue("NKNNNNNL(Nl)(Nl)(Nl)",
-                         INTEGER_TO_PY(st->st_mode),
+                         BUP_LONGISH_TO_PY(st->st_mode),
                          (unsigned PY_LONG_LONG) st->st_ino,
-                         INTEGER_TO_PY(st->st_dev),
-                         INTEGER_TO_PY(st->st_nlink),
-                         INTEGER_TO_PY(st->st_uid),
-                         INTEGER_TO_PY(st->st_gid),
-                         INTEGER_TO_PY(st->st_rdev),
+                         BUP_LONGISH_TO_PY(st->st_dev),
+                         BUP_LONGISH_TO_PY(st->st_nlink),
+                         BUP_LONGISH_TO_PY(st->st_uid),
+                         BUP_LONGISH_TO_PY(st->st_gid),
+                         BUP_LONGISH_TO_PY(st->st_rdev),
                          (PY_LONG_LONG) st->st_size,
-                         INTEGER_TO_PY(st->st_atime),
+                         BUP_LONGISH_TO_PY(st->st_atime),
                          (long) BUP_STAT_ATIME_NS(st),
-                         INTEGER_TO_PY(st->st_mtime),
+                         BUP_LONGISH_TO_PY(st->st_mtime),
                          (long) BUP_STAT_MTIME_NS(st),
-                         INTEGER_TO_PY(st->st_ctime),
+                         BUP_LONGISH_TO_PY(st->st_ctime),
                          (long) BUP_STAT_CTIME_NS(st));
 }
 
@@ -1573,15 +1570,15 @@ static PyObject *pwd_struct_to_py(const struct passwd *pwd)
 {
     // We can check the known (via POSIX) signed and unsigned types at
     // compile time, but not (easily) the unspecified types, so handle
-    // those via INTEGER_TO_PY().
+    // those via BUP_LONGISH_TO_PY().
     if (pwd == NULL)
         Py_RETURN_NONE;
     return Py_BuildValue(cstr_argf cstr_argf "OO"
                          cstr_argf cstr_argf cstr_argf,
                          pwd->pw_name,
                          pwd->pw_passwd,
-                         INTEGER_TO_PY(pwd->pw_uid),
-                         INTEGER_TO_PY(pwd->pw_gid),
+                         BUP_LONGISH_TO_PY(pwd->pw_uid),
+                         BUP_LONGISH_TO_PY(pwd->pw_gid),
                          pwd->pw_gecos,
                          pwd->pw_dir,
                          pwd->pw_shell);
@@ -1621,7 +1618,7 @@ static PyObject *grp_struct_to_py(const struct group *grp)
 {
     // We can check the known (via POSIX) signed and unsigned types at
     // compile time, but not (easily) the unspecified types, so handle
-    // those via INTEGER_TO_PY().
+    // those via BUP_LONGISH_TO_PY().
     if (grp == NULL)
         Py_RETURN_NONE;
 
@@ -1631,7 +1628,7 @@ static PyObject *grp_struct_to_py(const struct group *grp)
     return Py_BuildValue(cstr_argf cstr_argf "OO",
                          grp->gr_name,
                          grp->gr_passwd,
-                         INTEGER_TO_PY(grp->gr_gid),
+                         BUP_LONGISH_TO_PY(grp->gr_gid),
                          members);
 }
 
@@ -2274,7 +2271,7 @@ static int setup_module(PyObject *m)
     // Just be sure (relevant when passing timestamps back to Python above).
     assert(sizeof(PY_LONG_LONG) <= sizeof(long long));
     assert(sizeof(unsigned PY_LONG_LONG) <= sizeof(unsigned long long));
-    // At least for INTEGER_TO_PY
+    // At least for BUP_LONGISH_TO_PY
     assert(sizeof(intmax_t) <= sizeof(long long));
     assert(sizeof(uintmax_t) <= sizeof(unsigned long long));
     // This should be guaranteed by the C standard, but it's cheap to
@@ -2296,10 +2293,10 @@ static int setup_module(PyObject *m)
     char *e;
     {
         PyObject *value;
-        value = INTEGER_TO_PY(INT_MAX);
+        value = BUP_LONGISH_TO_PY(INT_MAX);
         PyObject_SetAttrString(m, "INT_MAX", value);
         Py_DECREF(value);
-        value = INTEGER_TO_PY(UINT_MAX);
+        value = BUP_LONGISH_TO_PY(UINT_MAX);
         PyObject_SetAttrString(m, "UINT_MAX", value);
         Py_DECREF(value);
     }
@@ -2308,13 +2305,13 @@ static int setup_module(PyObject *m)
 #ifdef HAVE_UTIMENSAT
     {
         PyObject *value;
-        value = INTEGER_TO_PY(AT_FDCWD);
+        value = BUP_LONGISH_TO_PY(AT_FDCWD);
         PyObject_SetAttrString(m, "AT_FDCWD", value);
         Py_DECREF(value);
-        value = INTEGER_TO_PY(AT_SYMLINK_NOFOLLOW);
+        value = BUP_LONGISH_TO_PY(AT_SYMLINK_NOFOLLOW);
         PyObject_SetAttrString(m, "AT_SYMLINK_NOFOLLOW", value);
         Py_DECREF(value);
-        value = INTEGER_TO_PY(UTIME_NOW);
+        value = BUP_LONGISH_TO_PY(UTIME_NOW);
         PyObject_SetAttrString(m, "UTIME_NOW", value);
         Py_DECREF(value);
     }
@@ -2324,7 +2321,7 @@ static int setup_module(PyObject *m)
 #ifdef BUP_HAVE_MINCORE_INCORE
     {
         PyObject *value;
-        value = INTEGER_TO_PY(MINCORE_INCORE);
+        value = BUP_LONGISH_TO_PY(MINCORE_INCORE);
         PyObject_SetAttrString(m, "MINCORE_INCORE", value);
         Py_DECREF(value);
     }
diff --git a/src/bup/pyutil.h b/src/bup/pyutil.h
index 680ca5460..d49e6f001 100644
--- a/src/bup/pyutil.h
+++ b/src/bup/pyutil.h
@@ -1,4 +1,11 @@
 #pragma once
 
+#include <sys/types.h>
+
+#include "bup/intprops.h"
+
+#define BUP_LONGISH_TO_PY(x) \
+    EXPR_SIGNED(x) ? PyLong_FromLongLong(x) : PyLong_FromUnsignedLongLong(x)
+
 void *checked_calloc(size_t n, size_t size);
 void *checked_malloc(size_t n, size_t size);
-- 
2.39.2

[PATCH 4/8] pyutil: add bup_uint_from_py bup_ulong_from_py bup_ullong_from_py

[Rob Browning]

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/_helpers.c | 58 ----------------------------------------------
 src/bup/pyutil.c   | 56 ++++++++++++++++++++++++++++++++++++++++++++
 src/bup/pyutil.h   |  4 ++++
 3 files changed, 60 insertions(+), 58 deletions(-)

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index d58cf9954..275ba0171 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -126,64 +126,6 @@ static uint64_t htonll(uint64_t value)
 #define INTEGRAL_ASSIGNMENT_FITS(dest, src) INT_ADD_OK(src, 0, dest)
 
 
-static int bup_ulong_from_py(unsigned long *x, PyObject *py, const char *name)
-{
-    if (!PyLong_Check(py))
-    {
-        PyErr_Format(PyExc_TypeError, "expected integer %s", name);
-        return 0;
-    }
-
-    const unsigned long tmp = PyLong_AsUnsignedLong(py);
-    if (PyErr_Occurred())
-    {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError))
-            PyErr_Format(PyExc_OverflowError, "%s too big for unsigned long",
-                         name);
-        return 0;
-    }
-    *x = tmp;
-    return 1;
-}
-
-
-static int bup_uint_from_py(unsigned int *x, PyObject *py, const char *name)
-{
-    unsigned long tmp;
-    if (!bup_ulong_from_py(&tmp, py, name))
-        return 0;
-
-    if (tmp > UINT_MAX)
-    {
-        PyErr_Format(PyExc_OverflowError, "%s too big for unsigned int", name);
-        return 0;
-    }
-    *x = (unsigned int) tmp;
-    return 1;
-}
-
-static int bup_ullong_from_py(unsigned PY_LONG_LONG *x, PyObject *py,
-                              const char *name)
-{
-    if (!PyLong_Check(py))
-    {
-        PyErr_Format(PyExc_TypeError, "integer argument expected for %s", name);
-        return 0;
-    }
-
-    const unsigned PY_LONG_LONG tmp = PyLong_AsUnsignedLongLong(py);
-    if (tmp == (unsigned long long) -1 && PyErr_Occurred())
-    {
-        if (PyErr_ExceptionMatches(PyExc_OverflowError))
-            PyErr_Format(PyExc_OverflowError,
-                         "%s too big for unsigned long long", name);
-        return 0;
-    }
-    *x = tmp;
-    return 1;
-}
-
-
 static PyObject *bup_bytescmp(PyObject *self, PyObject *args)
 {
     PyObject *py_s1, *py_s2;  // This is really a PyBytes/PyString
diff --git a/src/bup/pyutil.c b/src/bup/pyutil.c
index 53ca39c1d..5c480a4d3 100644
--- a/src/bup/pyutil.c
+++ b/src/bup/pyutil.c
@@ -35,3 +35,59 @@ void *checked_malloc(size_t n, size_t size)
         return PyErr_NoMemory();
     return result;
 }
+
+int bup_ulong_from_py(unsigned long *x, PyObject *py, const char *name)
+{
+    if (!PyLong_Check(py))
+    {
+        PyErr_Format(PyExc_TypeError, "%s expected integer, not %R", name, py);
+        return 0;
+    }
+
+    const unsigned long tmp = PyLong_AsUnsignedLong(py);
+    if (PyErr_Occurred())
+    {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError))
+            PyErr_Format(PyExc_OverflowError, "%s overflows unsigned long: %R",
+                         name, py);
+        return 0;
+    }
+    *x = tmp;
+    return 1;
+}
+
+int bup_uint_from_py(unsigned int *x, PyObject *py, const char *name)
+{
+    unsigned long tmp;
+    if (!bup_ulong_from_py(&tmp, py, name))
+        return 0;
+
+    if (tmp > UINT_MAX)
+    {
+        PyErr_Format(PyExc_OverflowError, "%s overflows unsigned int: %R",
+                     name, py);
+        return 0;
+    }
+    *x = (unsigned int) tmp;
+    return 1;
+}
+
+int bup_ullong_from_py(unsigned PY_LONG_LONG *x, PyObject *py, const char *name)
+{
+    if (!PyLong_Check(py))
+    {
+        PyErr_Format(PyExc_TypeError, "%s expected integer, not %R", name, py);
+        return 0;
+    }
+
+    const unsigned PY_LONG_LONG tmp = PyLong_AsUnsignedLongLong(py);
+    if (tmp == (unsigned long long) -1 && PyErr_Occurred())
+    {
+        if (PyErr_ExceptionMatches(PyExc_OverflowError))
+            PyErr_Format(PyExc_OverflowError,
+                         "%s overflows unsigned long long: %R", name, py);
+        return 0;
+    }
+    *x = tmp;
+    return 1;
+}
diff --git a/src/bup/pyutil.h b/src/bup/pyutil.h
index d49e6f001..4ea99d683 100644
--- a/src/bup/pyutil.h
+++ b/src/bup/pyutil.h
@@ -9,3 +9,7 @@
 
 void *checked_calloc(size_t n, size_t size);
 void *checked_malloc(size_t n, size_t size);
+
+int bup_uint_from_py(unsigned int *x, PyObject *py, const char *name);
+int bup_ulong_from_py(unsigned long *x, PyObject *py, const char *name);
+int bup_ullong_from_py(unsigned long long *x, PyObject *py, const char *name);
-- 
2.39.2

[PATCH 5/8] pyutil: add BUP_ASSIGN_PYLONG_TO_INTEGRAL; use EXPR_SIGNED

[Rob Browning]

Check EXPR_SIGNED to simplify the handling, i.e. we know which path
we're on up front.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/_helpers.c | 43 ++++---------------------------------------
 src/bup/pyutil.h   | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 275ba0171..2ca4ef839 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -1117,41 +1117,6 @@ static PyObject *bup_set_linux_file_attr(PyObject *self, PyObject *args)
 #endif
 #endif // defined BUP_USE_PYTHON_UTIME
 
-#define ASSIGN_PYLONG_TO_INTEGRAL(dest, pylong, overflow) \
-    ({                                                     \
-        int result = 0;                                                 \
-        *(overflow) = 0;                                                \
-        const long long lltmp = PyLong_AsLongLong(pylong);              \
-        if (lltmp == -1 && PyErr_Occurred())                            \
-        {                                                               \
-            if (PyErr_ExceptionMatches(PyExc_OverflowError))            \
-            {                                                           \
-                const unsigned long long ulltmp = PyLong_AsUnsignedLongLong(pylong); \
-                if (ulltmp == (unsigned long long) -1 && PyErr_Occurred()) \
-                {                                                       \
-                    if (PyErr_ExceptionMatches(PyExc_OverflowError))    \
-                    {                                                   \
-                        PyErr_Clear();                                  \
-                        *(overflow) = 1;                                \
-                    }                                                   \
-                }                                                       \
-                if (INTEGRAL_ASSIGNMENT_FITS((dest), ulltmp))           \
-                    result = 1;                                         \
-                else                                                    \
-                    *(overflow) = 1;                                    \
-            }                                                           \
-        }                                                               \
-        else                                                            \
-        {                                                               \
-            if (INTEGRAL_ASSIGNMENT_FITS((dest), lltmp))                \
-                result = 1;                                             \
-            else                                                        \
-                *(overflow) = 1;                                        \
-        }                                                               \
-        result;                                                         \
-        })
-
-
 #ifndef BUP_USE_PYTHON_UTIME // just for Python 2 now
 #ifdef HAVE_UTIMENSAT
 
@@ -1172,14 +1137,14 @@ static PyObject *bup_utimensat(PyObject *self, PyObject *args)
         return NULL;
 
     int overflow;
-    if (!ASSIGN_PYLONG_TO_INTEGRAL(&(ts[0].tv_sec), access_py, &overflow))
+    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(ts[0].tv_sec), access_py, &overflow))
     {
         if (overflow)
             PyErr_SetString(PyExc_ValueError,
                             "unable to convert access time seconds for utimensat");
         return NULL;
     }
-    if (!ASSIGN_PYLONG_TO_INTEGRAL(&(ts[1].tv_sec), modification_py, &overflow))
+    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(ts[1].tv_sec), modification_py, &overflow))
     {
         if (overflow)
             PyErr_SetString(PyExc_ValueError,
@@ -1212,7 +1177,7 @@ static int bup_parse_xutimes_args(char **path,
         return 0;
 
     int overflow;
-    if (!ASSIGN_PYLONG_TO_INTEGRAL(&(tv[0].tv_sec), access_py, &overflow))
+    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(tv[0].tv_sec), access_py, &overflow))
     {
         if (overflow)
             PyErr_SetString(PyExc_ValueError, "unable to convert access time seconds to timeval");
@@ -1223,7 +1188,7 @@ static int bup_parse_xutimes_args(char **path,
         PyErr_SetString(PyExc_ValueError, "unable to convert access time nanoseconds to timeval");
         return 0;
     }
-    if (!ASSIGN_PYLONG_TO_INTEGRAL(&(tv[1].tv_sec), modification_py, &overflow))
+    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(tv[1].tv_sec), modification_py, &overflow))
     {
         if (overflow)
             PyErr_SetString(PyExc_ValueError, "unable to convert modification time seconds to timeval");
diff --git a/src/bup/pyutil.h b/src/bup/pyutil.h
index 4ea99d683..37f4a100b 100644
--- a/src/bup/pyutil.h
+++ b/src/bup/pyutil.h
@@ -13,3 +13,42 @@ void *checked_malloc(size_t n, size_t size);
 int bup_uint_from_py(unsigned int *x, PyObject *py, const char *name);
 int bup_ulong_from_py(unsigned long *x, PyObject *py, const char *name);
 int bup_ullong_from_py(unsigned long long *x, PyObject *py, const char *name);
+
+// Currently only up to signed/unsigned long long given py api.  On
+// success returns non-zero.  On failure returns 0 and overflow will
+// be non-zero if there was an overflow, otherwise a python exception
+// will be pending.
+#define BUP_ASSIGN_PYLONG_TO_INTEGRAL (dest, pylong, overflow)          \
+    ({                                                                  \
+         int result = 0;                                                \
+         int pending_overflow = 0;                                      \
+         if (EXPR_SIGNED(dest)) {                                       \
+             const long long tmp = PyLong_AsLongLong(pylong);           \
+             if (tmp == -1 && PyErr_Occurred()                          \
+                 && PyErr_ExceptionMatches(PyExc_OverflowError))        \
+                 pending_overflow = 2;                                  \
+             else {                                                     \
+                 if (INTEGRAL_ASSIGNMENT_FITS((dest), tmp))             \
+                     result = 1;                                        \
+                 else                                                   \
+                     pending_overflow = 1;                              \
+             }                                                          \
+         } else {                                                       \
+             const unsigned long long tmp =                             \
+                 PyLong_AsUnsignedLongLong(pylong);                     \
+             if (tmp == -1 && PyErr_Occurred()                          \
+                 && PyErr_ExceptionMatches(PyExc_OverflowError))        \
+                 pending_overflow = 2;                                  \
+             else {                                                     \
+                 if (INTEGRAL_ASSIGNMENT_FITS((dest), tmp))             \
+                     result = 1;                                        \
+                 else                                                   \
+                     pending_overflow = 1;                              \
+             }                                                          \
+         }                                                              \
+         if (pending_overflow == 2) {                                   \
+             PyErr_Clear();                                             \
+             *(overflow) = 1;                                           \
+         }                                                              \
+         result;                                                        \
+    })
-- 
2.39.2

[PATCH 6/8] _helpers: remove vestigial py2 utimes related code

[Rob Browning]

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/_helpers.c | 180 ---------------------------------------------
 1 file changed, 180 deletions(-)

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 2ca4ef839..9bfb5e35a 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -73,20 +73,6 @@
 #define BUP_HAVE_FILE_ATTRS 1
 #endif
 
-#if PY_MAJOR_VERSION > 2
-# define BUP_USE_PYTHON_UTIME 1
-#endif
-
-#ifndef BUP_USE_PYTHON_UTIME // just for Python 2 now
-/*
- * Check for incomplete UTIMENSAT support (NetBSD 6), and if so,
- * pretend we don't have it.
- */
-#if !defined(AT_FDCWD) || !defined(AT_SYMLINK_NOFOLLOW)
-#undef HAVE_UTIMENSAT
-#endif
-#endif // defined BUP_USE_PYTHON_UTIME
-
 #ifndef FS_NOCOW_FL
 // Of course, this assumes it's a bitfield value.
 #define FS_NOCOW_FL 0
@@ -1106,138 +1092,6 @@ static PyObject *bup_set_linux_file_attr(PyObject *self, PyObject *args)
 #endif /* def BUP_HAVE_FILE_ATTRS */
 
 
-#ifndef BUP_USE_PYTHON_UTIME // just for Python 2 now
-#ifndef HAVE_UTIMENSAT
-#ifndef HAVE_UTIMES
-#error "cannot find utimensat or utimes()"
-#endif
-#ifndef HAVE_LUTIMES
-#error "cannot find utimensat or lutimes()"
-#endif
-#endif
-#endif // defined BUP_USE_PYTHON_UTIME
-
-#ifndef BUP_USE_PYTHON_UTIME // just for Python 2 now
-#ifdef HAVE_UTIMENSAT
-
-static PyObject *bup_utimensat(PyObject *self, PyObject *args)
-{
-    int rc;
-    int fd, flag;
-    char *path;
-    PyObject *access_py, *modification_py;
-    struct timespec ts[2];
-
-    if (!PyArg_ParseTuple(args, "i" cstr_argf "((Ol)(Ol))i",
-                          &fd,
-                          &path,
-                          &access_py, &(ts[0].tv_nsec),
-                          &modification_py, &(ts[1].tv_nsec),
-                          &flag))
-        return NULL;
-
-    int overflow;
-    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(ts[0].tv_sec), access_py, &overflow))
-    {
-        if (overflow)
-            PyErr_SetString(PyExc_ValueError,
-                            "unable to convert access time seconds for utimensat");
-        return NULL;
-    }
-    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(ts[1].tv_sec), modification_py, &overflow))
-    {
-        if (overflow)
-            PyErr_SetString(PyExc_ValueError,
-                            "unable to convert modification time seconds for utimensat");
-        return NULL;
-    }
-    rc = utimensat(fd, path, ts, flag);
-    if (rc != 0)
-        return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
-
-    return Py_BuildValue("O", Py_None);
-}
-
-#endif /* def HAVE_UTIMENSAT */
-
-
-#if defined(HAVE_UTIMES) || defined(HAVE_LUTIMES)
-
-static int bup_parse_xutimes_args(char **path,
-                                  struct timeval tv[2],
-                                  PyObject *args)
-{
-    PyObject *access_py, *modification_py;
-    long long access_us, modification_us; // POSIX guarantees tv_usec is signed.
-
-    if (!PyArg_ParseTuple(args, cstr_argf "((OL)(OL))",
-                          path,
-                          &access_py, &access_us,
-                          &modification_py, &modification_us))
-        return 0;
-
-    int overflow;
-    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(tv[0].tv_sec), access_py, &overflow))
-    {
-        if (overflow)
-            PyErr_SetString(PyExc_ValueError, "unable to convert access time seconds to timeval");
-        return 0;
-    }
-    if (!INTEGRAL_ASSIGNMENT_FITS(&(tv[0].tv_usec), access_us))
-    {
-        PyErr_SetString(PyExc_ValueError, "unable to convert access time nanoseconds to timeval");
-        return 0;
-    }
-    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&(tv[1].tv_sec), modification_py, &overflow))
-    {
-        if (overflow)
-            PyErr_SetString(PyExc_ValueError, "unable to convert modification time seconds to timeval");
-        return 0;
-    }
-    if (!INTEGRAL_ASSIGNMENT_FITS(&(tv[1].tv_usec), modification_us))
-    {
-        PyErr_SetString(PyExc_ValueError, "unable to convert modification time nanoseconds to timeval");
-        return 0;
-    }
-    return 1;
-}
-
-#endif /* defined(HAVE_UTIMES) || defined(HAVE_LUTIMES) */
-
-
-#ifdef HAVE_UTIMES
-static PyObject *bup_utimes(PyObject *self, PyObject *args)
-{
-    char *path;
-    struct timeval tv[2];
-    if (!bup_parse_xutimes_args(&path, tv, args))
-        return NULL;
-    int rc = utimes(path, tv);
-    if (rc != 0)
-        return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
-    return Py_BuildValue("O", Py_None);
-}
-#endif /* def HAVE_UTIMES */
-
-
-#ifdef HAVE_LUTIMES
-static PyObject *bup_lutimes(PyObject *self, PyObject *args)
-{
-    char *path;
-    struct timeval tv[2];
-    if (!bup_parse_xutimes_args(&path, tv, args))
-        return NULL;
-    int rc = lutimes(path, tv);
-    if (rc != 0)
-        return PyErr_SetFromErrnoWithFilename(PyExc_OSError, path);
-
-    return Py_BuildValue("O", Py_None);
-}
-#endif /* def HAVE_LUTIMES */
-
-#endif // defined BUP_USE_PYTHON_UTIME
-
-
 #ifdef HAVE_STAT_ST_ATIM
 # define BUP_STAT_ATIME_NS(st) (st)->st_atim.tv_nsec
 # define BUP_STAT_MTIME_NS(st) (st)->st_mtim.tv_nsec
@@ -2056,23 +1910,6 @@ static PyMethodDef helper_methods[] = {
     { "set_linux_file_attr", bup_set_linux_file_attr, METH_VARARGS,
       "Set the Linux attributes for the given file." },
 #endif
-
-#ifndef BUP_USE_PYTHON_UTIME // just for Python 2 now
-#ifdef HAVE_UTIMENSAT
-    { "bup_utimensat", bup_utimensat, METH_VARARGS,
-      "Change path timestamps with nanosecond precision (POSIX)." },
-#endif
-#ifdef HAVE_UTIMES
-    { "bup_utimes", bup_utimes, METH_VARARGS,
-      "Change path timestamps with microsecond precision." },
-#endif
-#ifdef HAVE_LUTIMES
-    { "bup_lutimes", bup_lutimes, METH_VARARGS,
-      "Change path timestamps with microsecond precision;"
-      " don't follow symlinks." },
-#endif
-#endif // defined BUP_USE_PYTHON_UTIME
-
     { "stat", bup_stat, METH_VARARGS,
       "Extended version of stat." },
     { "lstat", bup_lstat, METH_VARARGS,
@@ -2208,23 +2045,6 @@ static int setup_module(PyObject *m)
         Py_DECREF(value);
     }
 
-#ifndef BUP_USE_PYTHON_UTIME // just for Python 2 now
-#ifdef HAVE_UTIMENSAT
-    {
-        PyObject *value;
-        value = BUP_LONGISH_TO_PY(AT_FDCWD);
-        PyObject_SetAttrString(m, "AT_FDCWD", value);
-        Py_DECREF(value);
-        value = BUP_LONGISH_TO_PY(AT_SYMLINK_NOFOLLOW);
-        PyObject_SetAttrString(m, "AT_SYMLINK_NOFOLLOW", value);
-        Py_DECREF(value);
-        value = BUP_LONGISH_TO_PY(UTIME_NOW);
-        PyObject_SetAttrString(m, "UTIME_NOW", value);
-        Py_DECREF(value);
-    }
-#endif
-#endif // defined BUP_USE_PYTHON_UTIME
-
 #ifdef BUP_HAVE_MINCORE_INCORE
     {
         PyObject *value;
-- 
2.39.2

[PATCH 7/8] HashSplitter_init: guard against bits/fanbits overflow

[Rob Browning]

Replace PyArg_ParseTuple "I" conversion (which ignores overflow) with
bup_uint_from_py.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/_hashsplit.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lib/bup/_hashsplit.c b/lib/bup/_hashsplit.c
index ade413181..02798708f 100644
--- a/lib/bup/_hashsplit.c
+++ b/lib/bup/_hashsplit.c
@@ -31,6 +31,7 @@
 
 #include "_hashsplit.h"
 #include "bup/intprops.h"
+#include "bup/pyutil.h"
 #include "bupsplit.h"
 
 #if defined(FS_IOC_GETFLAGS) && defined(FS_IOC_SETFLAGS)
@@ -294,11 +295,11 @@ static int HashSplitter_init(HashSplitter *self, PyObject *args, PyObject *kwds)
         "fanbits",
         NULL
      };
-    PyObject *files = NULL;
-    if (!PyArg_ParseTupleAndKeywords(args, kwds, "OI|OpI", argnames,
-                                     &files, &self->bits,
+    PyObject *files = NULL, *py_bits = NULL, *py_fanbits = NULL;
+    if (!PyArg_ParseTupleAndKeywords(args, kwds, "OO|OpO", argnames,
+                                     &files, &py_bits,
                                      &self->progress, &self->boundaries,
-                                     &self->fanbits))
+                                     &py_fanbits))
         goto error;
 
     self->files = PyObject_GetIter(files);
@@ -311,6 +312,8 @@ static int HashSplitter_init(HashSplitter *self, PyObject *args, PyObject *kwds)
     else
         Py_INCREF(self->progress);
 
+    if(py_bits && !bup_uint_from_py(&self->bits, py_bits, "HashSplitter(bits)"))
+        goto error;
     if (self->bits < 13 || self->bits > max_bits) {
         PyErr_Format(PyExc_ValueError,
                      "invalid bits value %d (must be in [%d, %d])",
@@ -318,6 +321,9 @@ static int HashSplitter_init(HashSplitter *self, PyObject *args, PyObject *kwds)
         goto error;
     }
 
+    if(py_fanbits && !bup_uint_from_py(&self->fanbits, py_fanbits,
+                                       "HashSplitter(fanbits)"))
+        goto error;
     if (!self->fanbits) {
         PyErr_Format(PyExc_ValueError, "fanbits must be non-zero");
         goto error;
-- 
2.39.2

[PATCH 8/8] Reject bup_getpwuid bup_getgrgid argument overflow

[Rob Browning]

Replace PyArg_ParseTuple "K" checks (which ignore overflow) with
BUP_ASSIGN_PYLONG_TO_INTEGRAL.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
---
 lib/bup/_helpers.c | 25 ++++++++++++++++---------
 src/bup/pyutil.h   |  4 ++--
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 9bfb5e35a..9a8e1555a 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -1347,13 +1347,16 @@ static PyObject *pwd_struct_to_py(const struct passwd *pwd)
 
 static PyObject *bup_getpwuid(PyObject *self, PyObject *args)
 {
-    unsigned long long py_uid;
-    if (!PyArg_ParseTuple(args, "K", &py_uid))
+    PyObject *py_uid = NULL;
+    if (!PyArg_ParseTuple(args, "O", &py_uid))
 	return NULL;
     uid_t uid;
-    if (!INTEGRAL_ASSIGNMENT_FITS(&uid, py_uid))
-        return PyErr_Format(PyExc_OverflowError, "uid too large for uid_t");
-
+    int overflow;
+    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&uid, py_uid, &overflow)) {
+        if (overflow)
+            return PyErr_Format(PyExc_OverflowError, "uid too large for uid_t");
+        return NULL;
+    }
     errno = 0;
     struct passwd *pwd = getpwuid(uid);
     if (!pwd && errno)
@@ -1395,12 +1398,16 @@ static PyObject *grp_struct_to_py(const struct group *grp)
 
 static PyObject *bup_getgrgid(PyObject *self, PyObject *args)
 {
-    unsigned long long py_gid;
-    if (!PyArg_ParseTuple(args, "K", &py_gid))
+    PyObject *py_gid = NULL;
+    if (!PyArg_ParseTuple(args, "O", &py_gid))
 	return NULL;
     gid_t gid;
-    if (!INTEGRAL_ASSIGNMENT_FITS(&gid, py_gid))
-        return PyErr_Format(PyExc_OverflowError, "gid too large for gid_t");
+    int overflow;
+    if (!BUP_ASSIGN_PYLONG_TO_INTEGRAL(&gid, py_gid, &overflow)) {
+        if (overflow)
+            return PyErr_Format(PyExc_OverflowError, "gid too large for gid_t");
+        return NULL;
+    }
 
     errno = 0;
     struct group *grp = getgrgid(gid);
diff --git a/src/bup/pyutil.h b/src/bup/pyutil.h
index 37f4a100b..93059e985 100644
--- a/src/bup/pyutil.h
+++ b/src/bup/pyutil.h
@@ -18,7 +18,7 @@ int bup_ullong_from_py(unsigned long long *x, PyObject *py, const char *name);
 // success returns non-zero.  On failure returns 0 and overflow will
 // be non-zero if there was an overflow, otherwise a python exception
 // will be pending.
-#define BUP_ASSIGN_PYLONG_TO_INTEGRAL (dest, pylong, overflow)          \
+#define BUP_ASSIGN_PYLONG_TO_INTEGRAL(dest, pylong, overflow)           \
     ({                                                                  \
          int result = 0;                                                \
          int pending_overflow = 0;                                      \
@@ -36,7 +36,7 @@ int bup_ullong_from_py(unsigned long long *x, PyObject *py, const char *name);
          } else {                                                       \
              const unsigned long long tmp =                             \
                  PyLong_AsUnsignedLongLong(pylong);                     \
-             if (tmp == -1 && PyErr_Occurred()                          \
+             if (tmp == (unsigned long long) -1 && PyErr_Occurred()     \
                  && PyErr_ExceptionMatches(PyExc_OverflowError))        \
                  pending_overflow = 2;                                  \
              else {                                                     \
-- 
2.39.2

Re: [PATCH 0/8] Move py C utility code to pyutil; guard overflow

[Rob Browning]

Rob Browning <rlb@defaultvalue.org> writes:

> Proposed for main.
>
> This starts moving some of the utility functions to src/bup/pyutil.*,
> so that we can use them everywhere, e.g. hashsplit, bupsplit...

Apologies.  Clearly the wrong git send-email address.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4