Re: [Virtio-fs] [PATCH v2 1/7] virtiofsd: Changed allocations of fuse_req to GLib functions
* Mahmoud Mandour (ma.mando...@gmail.com) wrote: > On Tue, Apr 20, 2021 at 9:03 PM Vivek Goyal wrote: > > > On Tue, Apr 20, 2021 at 05:46:36PM +0200, Mahmoud Mandour wrote: > > > Replaced the allocation and deallocation of fuse_req structs > > > using calloc()/free() call pairs to a GLib's g_try_new0() > > > and g_free(). > > > > Hi, > > > > What's the context of these patches. I see all of them are switching > > to glib functions. Why to do that? What's the advantage. > > > > Vivek > > > > > > > > Signed-off-by: Mahmoud Mandour > > > Reviewed-by: Stefan Hajnoczi > > > --- > > > tools/virtiofsd/fuse_lowlevel.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/tools/virtiofsd/fuse_lowlevel.c > > b/tools/virtiofsd/fuse_lowlevel.c > > > index 58e32fc963..812cef6ef6 100644 > > > --- a/tools/virtiofsd/fuse_lowlevel.c > > > +++ b/tools/virtiofsd/fuse_lowlevel.c > > > @@ -106,7 +106,7 @@ static void list_add_req(struct fuse_req *req, > > struct fuse_req *next) > > > static void destroy_req(fuse_req_t req) > > > { > > > pthread_mutex_destroy(>lock); > > > -free(req); > > > +g_free(req); > > > } > > > > > > void fuse_free_req(fuse_req_t req) > > > @@ -130,7 +130,7 @@ static struct fuse_req *fuse_ll_alloc_req(struct > > fuse_session *se) > > > { > > > struct fuse_req *req; > > > > > > -req = (struct fuse_req *)calloc(1, sizeof(struct fuse_req)); > > > +req = g_try_new0(struct fuse_req, 1); > > > if (req == NULL) { > > > fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate request\n"); > > > } else { > > > @@ -1684,7 +1684,7 @@ static struct fuse_req *check_interrupt(struct > > fuse_session *se, > > > if (curr->u.i.unique == req->unique) { > > > req->interrupted = 1; > > > list_del_req(curr); > > > -free(curr); > > > +g_free(curr); > > > return NULL; > > > } > > > } > > > -- > > > 2.25.1 > > > > > > ___ > > > Virtio-fs mailing list > > > virtio...@redhat.com > > > https://listman.redhat.com/mailman/listinfo/virtio-fs > > > > > Hello Vivek, > > Taken from the Qemu Coding Style document in the documentation: > "Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign APIs > is not allowed in the QEMU codebase. Instead of these routines, use the > GLib memory allocation routines > g_malloc/g_malloc0/g_new/g_new0/g_realloc/g_free or QEMU’s > qemu_memalign/qemu_blockalign/qemu_vfree APIs." > It's also in the bite-sized contributions page as a task. Yes I think generally that's OK; note that virtiofsd is a little unusual in that a lot of it is imported from an external library and then changed; so we've not done a lot of these type of qemu-speicific styles. Dave > Thanks, > Mahmoud > ___ > Virtio-fs mailing list > virtio...@redhat.com > https://listman.redhat.com/mailman/listinfo/virtio-fs -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Virtio-fs] [PATCH v2 1/7] virtiofsd: Changed allocations of fuse_req to GLib functions
On Tue, Apr 20, 2021 at 9:03 PM Vivek Goyal wrote: > On Tue, Apr 20, 2021 at 05:46:36PM +0200, Mahmoud Mandour wrote: > > Replaced the allocation and deallocation of fuse_req structs > > using calloc()/free() call pairs to a GLib's g_try_new0() > > and g_free(). > > Hi, > > What's the context of these patches. I see all of them are switching > to glib functions. Why to do that? What's the advantage. > > Vivek > > > > > Signed-off-by: Mahmoud Mandour > > Reviewed-by: Stefan Hajnoczi > > --- > > tools/virtiofsd/fuse_lowlevel.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tools/virtiofsd/fuse_lowlevel.c > b/tools/virtiofsd/fuse_lowlevel.c > > index 58e32fc963..812cef6ef6 100644 > > --- a/tools/virtiofsd/fuse_lowlevel.c > > +++ b/tools/virtiofsd/fuse_lowlevel.c > > @@ -106,7 +106,7 @@ static void list_add_req(struct fuse_req *req, > struct fuse_req *next) > > static void destroy_req(fuse_req_t req) > > { > > pthread_mutex_destroy(>lock); > > -free(req); > > +g_free(req); > > } > > > > void fuse_free_req(fuse_req_t req) > > @@ -130,7 +130,7 @@ static struct fuse_req *fuse_ll_alloc_req(struct > fuse_session *se) > > { > > struct fuse_req *req; > > > > -req = (struct fuse_req *)calloc(1, sizeof(struct fuse_req)); > > +req = g_try_new0(struct fuse_req, 1); > > if (req == NULL) { > > fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate request\n"); > > } else { > > @@ -1684,7 +1684,7 @@ static struct fuse_req *check_interrupt(struct > fuse_session *se, > > if (curr->u.i.unique == req->unique) { > > req->interrupted = 1; > > list_del_req(curr); > > -free(curr); > > +g_free(curr); > > return NULL; > > } > > } > > -- > > 2.25.1 > > > > ___ > > Virtio-fs mailing list > > virtio...@redhat.com > > https://listman.redhat.com/mailman/listinfo/virtio-fs > > Hello Vivek, Taken from the Qemu Coding Style document in the documentation: "Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign APIs is not allowed in the QEMU codebase. Instead of these routines, use the GLib memory allocation routines g_malloc/g_malloc0/g_new/g_new0/g_realloc/g_free or QEMU’s qemu_memalign/qemu_blockalign/qemu_vfree APIs." It's also in the bite-sized contributions page as a task. Thanks, Mahmoud
Re: [Virtio-fs] [PATCH v2 1/7] virtiofsd: Changed allocations of fuse_req to GLib functions
On Tue, Apr 20, 2021 at 05:46:36PM +0200, Mahmoud Mandour wrote: > Replaced the allocation and deallocation of fuse_req structs > using calloc()/free() call pairs to a GLib's g_try_new0() > and g_free(). Hi, What's the context of these patches. I see all of them are switching to glib functions. Why to do that? What's the advantage. Vivek > > Signed-off-by: Mahmoud Mandour > Reviewed-by: Stefan Hajnoczi > --- > tools/virtiofsd/fuse_lowlevel.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c > index 58e32fc963..812cef6ef6 100644 > --- a/tools/virtiofsd/fuse_lowlevel.c > +++ b/tools/virtiofsd/fuse_lowlevel.c > @@ -106,7 +106,7 @@ static void list_add_req(struct fuse_req *req, struct > fuse_req *next) > static void destroy_req(fuse_req_t req) > { > pthread_mutex_destroy(>lock); > -free(req); > +g_free(req); > } > > void fuse_free_req(fuse_req_t req) > @@ -130,7 +130,7 @@ static struct fuse_req *fuse_ll_alloc_req(struct > fuse_session *se) > { > struct fuse_req *req; > > -req = (struct fuse_req *)calloc(1, sizeof(struct fuse_req)); > +req = g_try_new0(struct fuse_req, 1); > if (req == NULL) { > fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate request\n"); > } else { > @@ -1684,7 +1684,7 @@ static struct fuse_req *check_interrupt(struct > fuse_session *se, > if (curr->u.i.unique == req->unique) { > req->interrupted = 1; > list_del_req(curr); > -free(curr); > +g_free(curr); > return NULL; > } > } > -- > 2.25.1 > > ___ > Virtio-fs mailing list > virtio...@redhat.com > https://listman.redhat.com/mailman/listinfo/virtio-fs