[PATCH 3/8] virtiofsd: Changed fuse_pollhandle allocation to GLib's functions

Mahmoud Mandour posted 8 patches 4 years, 8 months ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There is a newer version of this series
[PATCH 3/8] virtiofsd: Changed fuse_pollhandle allocation to GLib's functions
Posted by Mahmoud Mandour 4 years, 8 months ago
Changed allocation of fuse_pollhandle structs to GLib's g_new().

Removed the null checking as allocating such a small memory segment
should always succeed on a healthy system. Otherwise, the system
is already in a critical state.

Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
 tools/virtiofsd/fuse_lowlevel.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 66607100f2..45527ff703 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -1755,7 +1755,7 @@ static void do_ioctl(fuse_req_t req, fuse_ino_t nodeid,
 
 void fuse_pollhandle_destroy(struct fuse_pollhandle *ph)
 {
-    free(ph);
+    g_free(ph);
 }
 
 static void do_poll(fuse_req_t req, fuse_ino_t nodeid,
@@ -1778,11 +1778,7 @@ static void do_poll(fuse_req_t req, fuse_ino_t nodeid,
         struct fuse_pollhandle *ph = NULL;
 
         if (arg->flags & FUSE_POLL_SCHEDULE_NOTIFY) {
-            ph = malloc(sizeof(struct fuse_pollhandle));
-            if (ph == NULL) {
-                fuse_reply_err(req, ENOMEM);
-                return;
-            }
+            ph = g_new(struct fuse_pollhandle, 1);
             ph->kh = arg->kh;
             ph->se = req->se;
         }
-- 
2.25.1


Re: [PATCH 3/8] virtiofsd: Changed fuse_pollhandle allocation to GLib's functions
Posted by Stefan Hajnoczi 4 years, 8 months ago
On Fri, Mar 19, 2021 at 03:25:22PM +0200, Mahmoud Mandour wrote:
> Changed allocation of fuse_pollhandle structs to GLib's g_new().
> 
> Removed the null checking as allocating such a small memory segment
> should always succeed on a healthy system. Otherwise, the system
> is already in a critical state.
> 
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> ---
>  tools/virtiofsd/fuse_lowlevel.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index 66607100f2..45527ff703 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -1755,7 +1755,7 @@ static void do_ioctl(fuse_req_t req, fuse_ino_t nodeid,
>  
>  void fuse_pollhandle_destroy(struct fuse_pollhandle *ph)
>  {
> -    free(ph);
> +    g_free(ph);
>  }
>  
>  static void do_poll(fuse_req_t req, fuse_ino_t nodeid,
> @@ -1778,11 +1778,7 @@ static void do_poll(fuse_req_t req, fuse_ino_t nodeid,
>          struct fuse_pollhandle *ph = NULL;
>  
>          if (arg->flags & FUSE_POLL_SCHEDULE_NOTIFY) {
> -            ph = malloc(sizeof(struct fuse_pollhandle));
> -            if (ph == NULL) {
> -                fuse_reply_err(req, ENOMEM);
> -                return;
> -            }
> +            ph = g_new(struct fuse_pollhandle, 1);

If the out-of-memory handling code is already there then I don't think
it should be removed. How have you determined that all hope is lost for
virtiofsd if this malloc fails?

By the way, this is dead code since passthrough_ll.c does not implement
the ->poll() callback. It's there because the code comes from upstream
libfuse and the idea was to leave the code relatively unmodified to make
applying future updates from libfuse easier.

Stefan