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

[Andrew Cooper <andcooper@xxxxxxxxx>]

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

> +             return (NULL);
> +     }
> +
> +     return (PyObject *)fs;
> +}
> +
>  static PyObject *
>  fsimage_getbootstring(PyObject *o, PyObject *args)
>  {
> @@ -302,6 +326,13 @@ PyDoc_STRVAR(fsimage_open__doc__,
>      "offset - offset of file system within file image.\n"
>      "options - mount options string.\n");
>  
> +PyDoc_STRVAR(fsimage_fdopen__doc__,
> +    "fdopen(fd, [offset=off]) - Use the file provided by the given fd as a 
> filesystem image.\n"
> +    "\n"
> +    "fd - File descriptor to use.\n"
> +    "offset - offset of file system within file image.\n"
> +    "options - mount options string.\n");
> +
>  PyDoc_STRVAR(fsimage_getbootstring__doc__,
>      "getbootstring(fs) - Return the boot string needed for this file system "
>      "or NULL if none is needed.\n");
> @@ -315,6 +346,8 @@ static struct PyMethodDef fsimage_module_methods[] = {
>           METH_VARARGS, fsimage_init__doc__ },
>       { "open", (PyCFunction)fsimage_open,
>           METH_VARARGS|METH_KEYWORDS, fsimage_open__doc__ },
> +     { "fdopen", (PyCFunction)fsimage_fdopen,
> +         METH_VARARGS|METH_KEYWORDS, fsimage_fdopen__doc__ },
>       { "getbootstring", (PyCFunction)fsimage_getbootstring,
>           METH_VARARGS, fsimage_getbootstring__doc__ },
>       { NULL, NULL, 0, NULL }