[libvirt] [libvirt-python PATCH 02/23] refactor the function to not override python exceptions

Pavel Hrdina phrdina at redhat.com
Tue Sep 29 14:09:49 UTC 2015


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 at 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) */
> >  
> > 




More information about the libvir-list mailing list