[PR PATCH] Bulk octal and exception fixup changes
Douglas Bagnall
douglas.bagnall at catalyst.net.nz
Wed Feb 28 03:41:29 UTC 2018
On 27/02/18 04:06, Github bot account via samba-technical wrote:
> There is a new pull request by noelpower against master on the Samba Samba Github repository
>
> https://github.com/noelpower/samba bulk_octal_and_except
> https://github.com/samba-team/samba/pull/135
>
> Bulk octal and exception fixup changes
> This patch converts literal octal numbers to a format that is acceptable in both python2 and python3.
> e.g.
>
> convert
> os.chmod(some_path, 0770) # valid in python2 only
> os.chmod(some_path, 0o770) # valid in python2 and python3
This bit is still good.
> additionally this pull request contains a patches to do a bulk
> conversion where except clause specifies a tuple following the
> exception name. This format is illegal in python3 e.g.
>
> convert
>
> except LdbError, (num, msg):
> to
> except LdbError as e:
> (num, msg) = e.args
Here there are a few mistakes where a comment ends up being duplicated:
> Subject: [PATCH 09/10] dsdb python tests: convert 'except X, (tuple)' to
> --- a/source4/dsdb/tests/python/passwords.py
> +++ b/source4/dsdb/tests/python/passwords.py
> @@ -340,7 +350,9 @@ def test_clearTextPassword_clear_set(self):
> FLAG_MOD_REPLACE, "clearTextPassword")
> self.ldb.modify(m)
> # this passes against s4
> - except LdbError, (num, msg):
> + except LdbError as e10:
> + # "NO_SUCH_ATTRIBUTE" is returned by Windows -> ignore it
> + (num, msg) = e10.args
> # "NO_SUCH_ATTRIBUTE" is returned by Windows -> ignore it
> if num != ERR_NO_SUCH_ATTRIBUTE:
> raise LdbError(num, msg)
which I have fixed manually.
I suspect you agree that the proliferation of exception variables
("e10" etc, rather than all "e") is unfortunate, but I know it would
be tricky to ensure all "e" was safe in an automatic process. And it
is good to get this done, so I won't block it those grounds. But this
is about the limit: we don't want automated changes whose results are
more inhuman than this.
(What I would *really* like is for LdbError to grow attributes like
IOError's errno and strerror, so the above could be:
except LdbError as e:
# "NO_SUCH_ATTRIBUTE" is returned by Windows -> ignore it
if e.errno != ERR_NO_SUCH_ATTRIBUTE:
raise
but that is not relevant to this patch.)
The last line of the first patch's commit message is slightly worrying:
> Subject: [PATCH 01/10] s4/dsdb: python3 api should take 'bytes'
>
> Attributes are properly represented by 'bytes' and *maybe* can be converted
> into strings (if they are text). py_dsdb_normalise_attributes
> currently expects strings, this is fine in python2 however in python3 we
> need to actually pass a 'bytes' class.
>
> Signed-off-by: Noel Power <noel.power at suse.com>
>
> squash
Did you mean to submit it in this form?
> - if (!PyStr_Check(str)) {
> - PyErr_SetString(PyExc_TypeError, "Expected a string argument to GUID()");
> + /* in python2 we get a string, in python3 we expect bytes */
> + if (!PyBytes_Check(str)) {
> + PyErr_SetString(PyExc_TypeError,
> + "Expected a "
> +#if IS_PY3
> + "bytes "
> +#else
> + "string "
> +#endif
> + "argument to GUID()");
I think I would prefer something like the attached patch, which
concentrates the #ifdef-ing in one place.
cheers,
Douglas
-------------- next part --------------
From 01a83fcbcac421b18bd4f655fe1fd212e2fd35c3 Mon Sep 17 00:00:00 2001
From: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date: Wed, 28 Feb 2018 15:59:06 +1300
Subject: [PATCH 1/3] py3compat: add strings describing bytes/unicode in both
versions
What Python 3 calls "bytes", Python 2 calls "string";
What Python 3 calls "string", Python 2 calls "unicode".
This can cause confusion in e.g. help strings where the precise type
matters. These macros can be used to construct accurate messages for
both versions.
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
python/py3compat.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/python/py3compat.h b/python/py3compat.h
index f54b3912f17..ce317c65f8e 100644
--- a/python/py3compat.h
+++ b/python/py3compat.h
@@ -78,6 +78,10 @@
#define PyStr_AsUTF8 PyUnicode_AsUTF8
#define PyStr_AsUTF8AndSize PyUnicode_AsUTF8AndSize
+/* description of bytes and string objects */
+#define PY_DESC_PY3_BYTES "bytes"
+#define PY_DESC_PY3_STRING "string"
+
/* Ints */
#define PyInt_Type PyLong_Type
@@ -144,6 +148,10 @@
#define PyBytes_ConcatAndDel PyString_ConcatAndDel
#define _PyBytes_Resize _PyString_Resize
+/* description of bytes and string objects */
+#define PY_DESC_PY3_BYTES "string"
+#define PY_DESC_PY3_STRING "unicode"
+
/* PyArg_ParseTuple/Py_BuildValue argument */
#define PYARG_BYTES_LEN "s#"
--
2.14.1
From 9c4a564d910686335dcf25b036214d0f8373ac99 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power at suse.com>
Date: Thu, 22 Feb 2018 12:49:36 +0000
Subject: [PATCH 2/3] s4/dsdb: python3 api should take 'bytes'
Attributes are properly represented by 'bytes' and *maybe* can be
converted into strings (if they are text).
py_dsdb_normalise_attributes currently expects strings, this is fine
in python2 however in python3 we need to actually pass a 'bytes'
class.
Signed-off-by: Noel Power <noel.power at suse.com>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
source4/dsdb/pydsdb.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c
index d38d7095efa..8d84a16dd18 100644
--- a/source4/dsdb/pydsdb.c
+++ b/source4/dsdb/pydsdb.c
@@ -625,7 +625,6 @@ static PyObject *py_dsdb_normalise_attributes(PyObject *self, PyObject *args)
TALLOC_CTX *tmp_ctx;
WERROR werr;
Py_ssize_t i;
- Py_ssize_t _size;
PyTypeObject *py_type = NULL;
PyObject *module = NULL;
@@ -688,13 +687,16 @@ static PyObject *py_dsdb_normalise_attributes(PyObject *self, PyObject *args)
for (i = 0; i < el->num_values; i++) {
PyObject *item = PyList_GetItem(el_list, i);
- if (!PyStr_Check(item)) {
- PyErr_Format(PyExc_TypeError, "ldif_elements should be strings");
+ if (!PyBytes_Check(item)) {
+ PyErr_Format(PyExc_TypeError,
+ "ldif_element type should be "
+ PY_DESC_PY3_BYTES
+ );
talloc_free(tmp_ctx);
return NULL;
}
- el->values[i].data = (uint8_t *)PyStr_AsUTF8AndSize(item, &_size);;
- el->values[i].length = _size;
+ el->values[i].data = (uint8_t *)PyBytes_AsString(item);
+ el->values[i].length = PyBytes_Size(item);
}
}
--
2.14.1
From 9d0b9da01e2f232a9f908667c5874cbaf26d71a9 Mon Sep 17 00:00:00 2001
From: Noel Power <noel.power at suse.com>
Date: Wed, 28 Feb 2018 16:25:55 +1300
Subject: [PATCH 3/3] s4/librpc: GUID should take a string in python2 and bytes
in python3
In python3 you can't store a binary blob GUID in a string class, you
need to use 'bytes'. This change ensure python2 code continues to use
a string and in python3 'bytes' are expected
Signed-off-by: Noel Power <noel.power at suse.com>
Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
---
source4/librpc/ndr/py_misc.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/source4/librpc/ndr/py_misc.c b/source4/librpc/ndr/py_misc.c
index 6316b9b6704..4e3efcfb709 100644
--- a/source4/librpc/ndr/py_misc.c
+++ b/source4/librpc/ndr/py_misc.c
@@ -95,14 +95,17 @@ static int py_GUID_init(PyObject *self, PyObject *args, PyObject *kwargs)
if (str != NULL) {
DATA_BLOB guid_val;
- Py_ssize_t _size;
- if (!PyStr_Check(str)) {
- PyErr_SetString(PyExc_TypeError, "Expected a string argument to GUID()");
+ /* in python2 we get a string, in python3 we expect bytes */
+ if (!PyBytes_Check(str)) {
+ PyErr_SetString(PyExc_TypeError,
+ "Expected a "
+ PY_DESC_PY3_BYTES
+ " argument to GUID()");
return -1;
}
- guid_val.data = (uint8_t *)PyStr_AsUTF8AndSize(str, &_size);
- guid_val.length = _size;
+ guid_val.data = (uint8_t *)PyBytes_AsString(str);
+ guid_val.length = PyBytes_Size(str);
status = GUID_from_data_blob(&guid_val, guid);
if (!NT_STATUS_IS_OK(status)) {
PyErr_SetNTSTATUS(status);
--
2.14.1
More information about the samba-technical
mailing list