Re: [libvirt] [libvirt-python PATCH 02/23] refactor the function to not override python exceptions
On Sat, Sep 26, 2015 at 09:08:11AM -0400, John Ferlan wrote:
On 09/24/2015 10:01 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
> ---
> libvirt-override.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/libvirt-override.c b/libvirt-override.c
> index 4dfe332..14aa0e9 100644
> --- a/libvirt-override.c
> +++ b/libvirt-override.c
> @@ -8263,38 +8263,31 @@ libvirt_virDomainSetTime(PyObject *self ATTRIBUTE_UNUSED,
PyObject *args) {
Seems as though py_retval can be removed as it's no longer needed (build
whine)
It seems, that I forgot to remove the declaration of unused variables through
the whole series. Thanks for noticing that, I didn't get any error during
compilation.
> if (PyDict_Check(py_dict)) {
> py_dict_size = PyDict_Size(py_dict);
> if ((pyobj_seconds = PyDict_GetItemString(py_dict, "seconds")))
{
> - if (libvirt_longlongUnwrap(pyobj_seconds, &seconds) < 0) {
> - PyErr_Format(PyExc_LookupError, "malformed
'seconds'");
> - goto cleanup;
> - }
> + if (libvirt_longlongUnwrap(pyobj_seconds, &seconds) < 0)
> + return NULL;
So if this was missing - would the error indicate 'seconds' was
malformed? Or just some generic invalid value python error? I'm
preferential to specific errors that help point me at what I did wrong.
Well, the reason, that the unwrap function fails don't have to always be
relevant to the "malformed 'second'" and it could mislead the user that
it's his
fault, but it could be anything else. For example, the unwrap function checks,
whether the pyobj_seconds is number or not.
Pavel
John
> } else {
> PyErr_Format(PyExc_LookupError, "Dictionary must contains
'seconds'");
> - goto cleanup;
> + return NULL;
> }
>
> if ((pyobj_nseconds = PyDict_GetItemString(py_dict, "nseconds")))
{
> - if (libvirt_uintUnwrap(pyobj_nseconds, &nseconds) < 0) {
> - PyErr_Format(PyExc_LookupError, "malformed
'nseconds'");
> - goto cleanup;
> - }
> + if (libvirt_uintUnwrap(pyobj_nseconds, &nseconds) < 0)
> + return NULL;
> } else if (py_dict_size > 1) {
> PyErr_Format(PyExc_LookupError, "Dictionary contains unknown
key");
> - goto cleanup;
> + return NULL;
> }
> } else if (py_dict != Py_None || !flags) {
> PyErr_Format(PyExc_TypeError, "time must be a dictionary "
> "or None with flags set");
> - goto cleanup;
> + return NULL;
> }
>
> LIBVIRT_BEGIN_ALLOW_THREADS;
> c_retval = virDomainSetTime(domain, seconds, nseconds, flags);
> LIBVIRT_END_ALLOW_THREADS;
>
> - py_retval = libvirt_intWrap(c_retval);
> -
> - cleanup:
> - return py_retval;
> + return libvirt_intWrap(c_retval);
> }
> #endif /* LIBVIR_CHECK_VERSION(1, 2, 5) */
>
>