[libvirt] [python PATCH 0/4] Random fixes

Pavel Hrdina (4): libvirt-utils: remove unused py_str function typewrappers: Fix libvirt_charPtrUnwrap to set an exception if it fails libvirt-override: Reset exception if the error is ignored libvirt_charPtrUnwrap: remove unnecessary check of returned string libvirt-override.c | 16 +++++++--------- libvirt-utils.c | 24 +++--------------------- libvirt-utils.h | 1 - typewrappers.c | 5 ++++- 4 files changed, 14 insertions(+), 32 deletions(-) -- 2.17.1

Commit <57a160b5248ba47d4e1c9d22d95847dad8e0524f> removed last usage but did not remove the function itself. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-utils.c | 14 -------------- libvirt-utils.h | 1 - 2 files changed, 15 deletions(-) diff --git a/libvirt-utils.c b/libvirt-utils.c index f7b4478..e17e794 100644 --- a/libvirt-utils.c +++ b/libvirt-utils.c @@ -199,20 +199,6 @@ virTypedParamsFree(virTypedParameterPtr params, } #endif /* ! LIBVIR_CHECK_VERSION(1, 0, 2) */ -char * -py_str(PyObject *obj) -{ - PyObject *str = PyObject_Str(obj); - char *ret; - if (!str) { - PyErr_Print(); - PyErr_Clear(); - return NULL; - }; - libvirt_charPtrUnwrap(str, &ret); - return ret; -} - /* Helper function to convert a virTypedParameter output array into a * Python dictionary for return to the user. Return NULL on failure, * after raising a python exception. */ diff --git a/libvirt-utils.h b/libvirt-utils.h index 0af1e62..cc3d278 100644 --- a/libvirt-utils.h +++ b/libvirt-utils.h @@ -319,7 +319,6 @@ void virTypedParamsClear(virTypedParameterPtr params, int nparams); void virTypedParamsFree(virTypedParameterPtr params, int nparams); # endif /* ! LIBVIR_CHECK_VERSION(1, 0, 2) */ -char * py_str(PyObject *obj); PyObject * getPyVirTypedParameter(const virTypedParameter *params, int nparams); virTypedParameterPtr setPyVirTypedParameter(PyObject *info, -- 2.17.1

On Tue, Jun 12, 2018 at 07:16:38AM +0200, Pavel Hrdina wrote:
Commit <57a160b5248ba47d4e1c9d22d95847dad8e0524f> removed last usage but did not remove the function itself.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-utils.c | 14 -------------- libvirt-utils.h | 1 - 2 files changed, 15 deletions(-)
Beautiful diffstat. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

If the function fails it should always set an exception. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- typewrappers.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/typewrappers.c b/typewrappers.c index 01ee310..99a8fb8 100644 --- a/typewrappers.c +++ b/typewrappers.c @@ -384,8 +384,11 @@ libvirt_charPtrUnwrap(PyObject *obj, #else ret = PyString_AsString(obj); #endif - if (ret) + if (ret) { *str = strdup(ret); + if (!*str) + PyErr_NoMemory(); + } #if PY_MAJOR_VERSION > 2 Py_DECREF(bytes); #endif -- 2.17.1

On Tue, Jun 12, 2018 at 07:16:39AM +0200, Pavel Hrdina wrote:
If the function fails it should always set an exception.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- typewrappers.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

In virConnectCredCallbackWrapper() we ignore the error case of libvirt_charPtrUnwrap() function so we should also reset the exception. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-override.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libvirt-override.c b/libvirt-override.c index 1c95c18..dac481b 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -1918,8 +1918,10 @@ virConnectCredCallbackWrapper(virConnectCredentialPtr cred, char *result = NULL; pycreditem = PyTuple_GetItem(pycred, i); pyresult = PyList_GetItem(pycreditem, 4); - if (pyresult != Py_None) + if (pyresult != Py_None) { libvirt_charPtrUnwrap(pyresult, &result); + PyErr_Clear(); + } if (result != NULL) { cred[i].result = result; cred[i].resultlen = strlen(result); -- 2.17.1

On Tue, Jun 12, 2018 at 07:16:40AM +0200, Pavel Hrdina wrote:
In virConnectCredCallbackWrapper() we ignore the error case of libvirt_charPtrUnwrap() function so we should also reset the exception.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-override.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

Function libvirt_charPtrUnwrap() either fails or always sets the unwrapped string so there is no need to check it explicitly. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-override.c | 12 ++++-------- libvirt-utils.c | 10 +++------- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/libvirt-override.c b/libvirt-override.c index dac481b..2f2c4ff 100644 --- a/libvirt-override.c +++ b/libvirt-override.c @@ -4639,8 +4639,7 @@ libvirt_virConnectBaselineCPU(PyObject *self ATTRIBUTE_UNUSED, for (i = 0; i < ncpus; i++) { if (libvirt_charPtrUnwrap(PyList_GetItem(list, i), - &(xmlcpus[i])) < 0 || - xmlcpus[i] == NULL) { + &(xmlcpus[i])) < 0) { for (j = 0 ; j < i ; j++) VIR_FREE(xmlcpus[j]); VIR_FREE(xmlcpus); @@ -8245,8 +8244,7 @@ libvirt_virDomainFSFreeze(PyObject *self ATTRIBUTE_UNUSED, for (i = 0; i < nmountpoints; i++) { if (libvirt_charPtrUnwrap(PyList_GetItem(pyobj_list, i), - mountpoints+i) < 0 || - mountpoints[i] == NULL) + mountpoints+i) < 0) goto cleanup; } } @@ -8293,8 +8291,7 @@ libvirt_virDomainFSThaw(PyObject *self ATTRIBUTE_UNUSED, for (i = 0; i < nmountpoints; i++) { if (libvirt_charPtrUnwrap(PyList_GetItem(pyobj_list, i), - mountpoints+i) < 0 || - mountpoints[i] == NULL) + mountpoints+i) < 0) goto cleanup; } } @@ -9743,8 +9740,7 @@ libvirt_virConnectBaselineHypervisorCPU(PyObject *self ATTRIBUTE_UNUSED, for (i = 0; i < ncpus; i++) { if (libvirt_charPtrUnwrap(PyList_GetItem(list, i), - &(xmlCPUs[i])) < 0 || - !xmlCPUs[i]) + &(xmlCPUs[i])) < 0) goto cleanup; } } diff --git a/libvirt-utils.c b/libvirt-utils.c index e17e794..78b94ca 100644 --- a/libvirt-utils.c +++ b/libvirt-utils.c @@ -302,8 +302,7 @@ setPyVirTypedParameter(PyObject *info, while (PyDict_Next(info, &pos, &key, &value)) { char *keystr = NULL; - if (libvirt_charPtrUnwrap(key, &keystr) < 0 || - keystr == NULL) + if (libvirt_charPtrUnwrap(key, &keystr) < 0) goto cleanup; for (i = 0; i < nparams; i++) { @@ -359,8 +358,7 @@ setPyVirTypedParameter(PyObject *info, case VIR_TYPED_PARAM_STRING: { char *string_val; - if (libvirt_charPtrUnwrap(value, &string_val) < 0 || - !string_val) + if (libvirt_charPtrUnwrap(value, &string_val) < 0) goto cleanup; temp->value.s = string_val; break; @@ -489,7 +487,6 @@ virPyDictToTypedParamOne(virTypedParameterPtr *params, { char *val;; if (libvirt_charPtrUnwrap(value, &val) < 0 || - !val || virTypedParamsAddString(params, n, max, keystr, val) < 0) { VIR_FREE(val); goto cleanup; @@ -541,8 +538,7 @@ virPyDictToTypedParams(PyObject *dict, return -1; while (PyDict_Next(dict, &pos, &key, &value)) { - if (libvirt_charPtrUnwrap(key, &keystr) < 0 || - !keystr) + if (libvirt_charPtrUnwrap(key, &keystr) < 0) goto cleanup; if (PyList_Check(value) || PyTuple_Check(value)) { -- 2.17.1

On Tue, Jun 12, 2018 at 07:16:41AM +0200, Pavel Hrdina wrote:
Function libvirt_charPtrUnwrap() either fails or always sets the unwrapped string so there is no need to check it explicitly.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- libvirt-override.c | 12 ++++-------- libvirt-utils.c | 10 +++------- 2 files changed, 7 insertions(+), 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Pavel Hrdina