Re: [PATCH 5/6] tools/pygrub: Expose libfsimage's fdopen() to python

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 22/11/2023 22:35, Andrew Cooper wrote:
> On 06/11/2023 3:05 pm, Alejandro Vallejo wrote:
> > Create a wrapper for the new fdopen() function of libfsimage.
> >
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> I'd appreciate it if Marek would cast his eye (as python maintainer)
> over it.
>
> That said, ...
>
> > diff --git a/tools/pygrub/src/fsimage/fsimage.c 
> > b/tools/pygrub/src/fsimage/fsimage.c
> > index 12dfcff6e3..216f265331 100644
> > --- a/tools/pygrub/src/fsimage/fsimage.c
> > +++ b/tools/pygrub/src/fsimage/fsimage.c
> > @@ -270,6 +270,30 @@ fsimage_open(PyObject *o, PyObject *args, PyObject *kwargs)
> >         return (PyObject *)fs;
> >   }
> >   
> > +static PyObject *
> > +fsimage_fdopen(PyObject *o, PyObject *args, PyObject *kwargs)
> > +{
> > +       static char *kwlist[] = { "fd", "offset", "options", NULL };
> > +       int fd;
> > +       char *options = NULL;
> > +       uint64_t offset = 0;
> > +       fsimage_fs_t *fs;
> > +
> > +       if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|Ls", kwlist,
> > +           &fd, &offset, &options))
> > +               return (NULL);
> > +
> > +       if ((fs = PyObject_NEW(fsimage_fs_t, &fsimage_fs_type)) == NULL)
> > +               return (NULL);
> > +
> > +       if ((fs->fs = fsi_fdopen_fsimage(fd, offset, options)) == NULL) {
> > +               PyErr_SetFromErrno(PyExc_IOError);
> Don't we need a Py_DECREF(fs) here to avoid leaking it?
>
> ~Andrew
If so, there's a bug in fsimage_open() as well. The logic here identical
to the logic there.

Cheers,
Alejandro