diff mbox series

[8/8] tools/virtiofsd: Replacing malloc-like calls with GLib's variants

Message ID 20210314032324.45142-9-ma.mandourr@gmail.com (mailing list archive)
State New, archived
Headers show
Series Replacing malloc and the like with GLib's variants | expand

Commit Message

Mahmoud Mandour March 14, 2021, 3:23 a.m. UTC
Changed calls to malloc(), calloc(), and realloc() with their
equivalent allocation functions in GLib, and replaced their
respective free() calls with g_free().

Allocation and deallocation of fuse_req structs, fuse_pollhandle
structs, fuse_session structs and many local variables are now
established through GLib's functions.

Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
 tools/virtiofsd/fuse_lowlevel.c  | 30 ++++++++++++++--------------
 tools/virtiofsd/fuse_virtio.c    | 34 ++++++++++++++++----------------
 tools/virtiofsd/passthrough_ll.c | 32 +++++++++++++++---------------
 3 files changed, 48 insertions(+), 48 deletions(-)

Comments

Stefan Hajnoczi March 15, 2021, 10:01 a.m. UTC | #1
On Sun, Mar 14, 2021 at 05:23:24AM +0200, Mahmoud Mandour wrote:
> @@ -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_new(struct fuse_req, 1);

g_try_new0() since the original call was calloc(3)?

> @@ -411,7 +411,7 @@ static int lo_map_grow(struct lo_map *map, size_t new_nelems)
>          return 1;
>      }
>  
> -    new_elems = realloc(map->elems, sizeof(map->elems[0]) * new_nelems);
> +    new_elems = g_realloc_n(map->elems, new_nelems, sizeof(map->elems[0]));

g_try_realloc_n() since failure is handled below?

Stefan
Mahmoud Mandour March 15, 2021, 10:46 a.m. UTC | #2
On Mon, Mar 15, 2021 at 12:01 PM Stefan Hajnoczi <stefanha@redhat.com>
wrote:

> On Sun, Mar 14, 2021 at 05:23:24AM +0200, Mahmoud Mandour wrote:
> > @@ -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_new(struct fuse_req, 1);
>
> g_try_new0() since the original call was calloc(3)?
>
> > @@ -411,7 +411,7 @@ static int lo_map_grow(struct lo_map *map, size_t
> new_nelems)
> >          return 1;
> >      }
> >
> > -    new_elems = realloc(map->elems, sizeof(map->elems[0]) * new_nelems);
> > +    new_elems = g_realloc_n(map->elems, new_nelems,
> sizeof(map->elems[0]));
>
> g_try_realloc_n() since failure is handled below?
>
> Stefan
>

Hello Mr. Stefan,

You're correct. I'm really sorry for such small and strangely obvious
errors.
If the patch is going to be ACKed, will you edit those problems or shall I
fix them and
resend the patch again alone?
Stefan Hajnoczi March 16, 2021, 5:23 p.m. UTC | #3
On Mon, Mar 15, 2021 at 12:46:29PM +0200, Mahmoud Mandour wrote:
> On Mon, Mar 15, 2021 at 12:01 PM Stefan Hajnoczi <stefanha@redhat.com>
> wrote:
> 
> > On Sun, Mar 14, 2021 at 05:23:24AM +0200, Mahmoud Mandour wrote:
> > > @@ -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_new(struct fuse_req, 1);
> >
> > g_try_new0() since the original call was calloc(3)?
> >
> > > @@ -411,7 +411,7 @@ static int lo_map_grow(struct lo_map *map, size_t
> > new_nelems)
> > >          return 1;
> > >      }
> > >
> > > -    new_elems = realloc(map->elems, sizeof(map->elems[0]) * new_nelems);
> > > +    new_elems = g_realloc_n(map->elems, new_nelems,
> > sizeof(map->elems[0]));
> >
> > g_try_realloc_n() since failure is handled below?
> >
> > Stefan
> >
> 
> Hello Mr. Stefan,
> 
> You're correct. I'm really sorry for such small and strangely obvious
> errors.
> If the patch is going to be ACKed, will you edit those problems or shall I
> fix them and
> resend the patch again alone?

Hi,
Don't worry, it happens to everyone! Please resend.

Stefan
diff mbox series

Patch

diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 1aa26c6333..5e188f8d8f 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(&req->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_new(struct fuse_req, 1);
     if (req == NULL) {
         fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate request\n");
     } else {
@@ -219,7 +219,7 @@  int fuse_reply_iov(fuse_req_t req, const struct iovec *iov, int count)
     int res;
     struct iovec *padded_iov;
 
-    padded_iov = malloc((count + 1) * sizeof(struct iovec));
+    padded_iov = g_try_new(struct iovec, count + 1);
     if (padded_iov == NULL) {
         return fuse_reply_err(req, ENOMEM);
     }
@@ -228,7 +228,7 @@  int fuse_reply_iov(fuse_req_t req, const struct iovec *iov, int count)
     count++;
 
     res = send_reply_iov(req, 0, padded_iov, count);
-    free(padded_iov);
+    g_free(padded_iov);
 
     return res;
 }
@@ -568,7 +568,7 @@  static struct fuse_ioctl_iovec *fuse_ioctl_iovec_copy(const struct iovec *iov,
     struct fuse_ioctl_iovec *fiov;
     size_t i;
 
-    fiov = malloc(sizeof(fiov[0]) * count);
+    fiov = g_try_new(struct fuse_ioctl_iovec, count);
     if (!fiov) {
         return NULL;
     }
@@ -629,8 +629,8 @@  int fuse_reply_ioctl_retry(fuse_req_t req, const struct iovec *in_iov,
 
     res = send_reply_iov(req, 0, iov, count);
 out:
-    free(in_fiov);
-    free(out_fiov);
+    g_free(in_fiov);
+    g_free(out_fiov);
 
     return res;
 
@@ -667,7 +667,7 @@  int fuse_reply_ioctl_iov(fuse_req_t req, int result, const struct iovec *iov,
     struct fuse_ioctl_out arg;
     int res;
 
-    padded_iov = malloc((count + 2) * sizeof(struct iovec));
+    padded_iov = g_try_new(struct iovec, count + 2);
     if (padded_iov == NULL) {
         return fuse_reply_err(req, ENOMEM);
     }
@@ -680,7 +680,7 @@  int fuse_reply_ioctl_iov(fuse_req_t req, int result, const struct iovec *iov,
     memcpy(&padded_iov[2], iov, count * sizeof(struct iovec));
 
     res = send_reply_iov(req, 0, padded_iov, count + 2);
-    free(padded_iov);
+    g_free(padded_iov);
 
     return res;
 }
@@ -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;
         }
     }
@@ -1760,7 +1760,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,
@@ -1783,7 +1783,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));
+            ph = g_try_new(struct fuse_pollhandle, 1);
             if (ph == NULL) {
                 fuse_reply_err(req, ENOMEM);
                 return;
@@ -2476,7 +2476,7 @@  void fuse_session_destroy(struct fuse_session *se)
     free(se->vu_socket_path);
     se->vu_socket_path = NULL;
 
-    free(se);
+    g_free(se);
 }
 
 
@@ -2499,7 +2499,7 @@  struct fuse_session *fuse_session_new(struct fuse_args *args,
         return NULL;
     }
 
-    se = (struct fuse_session *)calloc(1, sizeof(struct fuse_session));
+    se = g_try_new0(struct fuse_session, 1);
     if (se == NULL) {
         fuse_log(FUSE_LOG_ERR, "fuse: failed to allocate fuse object\n");
         goto out1;
@@ -2559,7 +2559,7 @@  struct fuse_session *fuse_session_new(struct fuse_args *args,
 out4:
     fuse_opt_free_args(args);
 out2:
-    free(se);
+    g_free(se);
 out1:
     return NULL;
 }
diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 523ee64fb7..598c97db1f 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -347,7 +347,7 @@  int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
      * Build a copy of the the in_sg iov so we can skip bits in it,
      * including changing the offsets
      */
-    struct iovec *in_sg_cpy = calloc(sizeof(struct iovec), in_num);
+    struct iovec *in_sg_cpy = g_try_new0(struct iovec, in_num);
     assert(in_sg_cpy);
     memcpy(in_sg_cpy, in_sg, sizeof(struct iovec) * in_num);
     /* These get updated as we skip */
@@ -386,7 +386,7 @@  int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
             ret = errno;
             fuse_log(FUSE_LOG_DEBUG, "%s: preadv failed (%m) len=%zd\n",
                      __func__, len);
-            free(in_sg_cpy);
+            g_free(in_sg_cpy);
             goto err;
         }
         fuse_log(FUSE_LOG_DEBUG, "%s: preadv ret=%d len=%zd\n", __func__,
@@ -410,13 +410,13 @@  int virtio_send_data_iov(struct fuse_session *se, struct fuse_chan *ch,
         if (ret != len) {
             fuse_log(FUSE_LOG_DEBUG, "%s: ret!=len\n", __func__);
             ret = EIO;
-            free(in_sg_cpy);
+            g_free(in_sg_cpy);
             goto err;
         }
         in_sg_left -= ret;
         len -= ret;
     } while (in_sg_left);
-    free(in_sg_cpy);
+    g_free(in_sg_cpy);
 
     /* Need to fix out->len on EOF */
     if (len) {
@@ -476,7 +476,7 @@  static void fv_queue_worker(gpointer data, gpointer user_data)
      * They're spread over multiple descriptors in a scatter/gather set
      * and we can't trust the guest to keep them still; so copy in/out.
      */
-    fbuf.mem = malloc(se->bufsize);
+    fbuf.mem = g_try_malloc(se->bufsize);
     assert(fbuf.mem);
 
     fuse_mutex_init(&req->ch.lock);
@@ -528,10 +528,10 @@  static void fv_queue_worker(gpointer data, gpointer user_data)
         fbuf.size = out_sg[0].iov_len + out_sg[1].iov_len;
 
         /* Allocate the bufv, with space for the rest of the iov */
-        pbufv = malloc(sizeof(struct fuse_bufvec) +
+        pbufv = g_try_malloc(sizeof(struct fuse_bufvec) +
                        sizeof(struct fuse_buf) * (out_num - 2));
         if (!pbufv) {
-            fuse_log(FUSE_LOG_ERR, "%s: pbufv malloc failed\n",
+            fuse_log(FUSE_LOG_ERR, "%s: pbufv g_try_malloc failed\n",
                     __func__);
             goto out;
         }
@@ -573,7 +573,7 @@  static void fv_queue_worker(gpointer data, gpointer user_data)
 
 out:
     if (allocated_bufv) {
-        free(pbufv);
+        g_free(pbufv);
     }
 
     /* If the request has no reply, still recycle the virtqueue element */
@@ -592,8 +592,8 @@  out:
     }
 
     pthread_mutex_destroy(&req->ch.lock);
-    free(fbuf.mem);
-    free(req);
+    g_free(fbuf.mem);
+    g_free(req);
 }
 
 /* Thread function for individual queues, created when a queue is 'started' */
@@ -733,7 +733,7 @@  static void fv_queue_cleanup_thread(struct fv_VuDev *vud, int qidx)
     pthread_mutex_destroy(&ourqi->vq_lock);
     close(ourqi->kill_fd);
     ourqi->kick_fd = -1;
-    free(vud->qi[qidx]);
+    g_free(vud->qi[qidx]);
     vud->qi[qidx] = NULL;
 }
 
@@ -764,14 +764,14 @@  static void fv_queue_set_started(VuDev *dev, int qidx, bool started)
     if (started) {
         /* Fire up a thread to watch this queue */
         if (qidx >= vud->nqueues) {
-            vud->qi = realloc(vud->qi, (qidx + 1) * sizeof(vud->qi[0]));
+            vud->qi = g_try_realloc_n(vud->qi, (qidx + 1), sizeof(vud->qi[0]));
             assert(vud->qi);
             memset(vud->qi + vud->nqueues, 0,
                    sizeof(vud->qi[0]) * (1 + (qidx - vud->nqueues)));
             vud->nqueues = qidx + 1;
         }
         if (!vud->qi[qidx]) {
-            vud->qi[qidx] = calloc(sizeof(struct fv_QueueInfo), 1);
+            vud->qi[qidx] = g_try_new0(struct fv_QueueInfo, 1);
             assert(vud->qi[qidx]);
             vud->qi[qidx]->virtio_dev = vud;
             vud->qi[qidx]->qidx = qidx;
@@ -1032,9 +1032,9 @@  int virtio_session_mount(struct fuse_session *se)
              __func__);
 
     /* TODO: Some cleanup/deallocation! */
-    se->virtio_dev = calloc(sizeof(struct fv_VuDev), 1);
+    se->virtio_dev = g_try_new0(struct fv_VuDev, 1);
     if (!se->virtio_dev) {
-        fuse_log(FUSE_LOG_ERR, "%s: virtio_dev calloc failed\n", __func__);
+        fuse_log(FUSE_LOG_ERR, "%s: virtio_dev g_try_new0 failed\n", __func__);
         close(data_sock);
         return -1;
     }
@@ -1059,8 +1059,8 @@  void virtio_session_close(struct fuse_session *se)
         return;
     }
 
-    free(se->virtio_dev->qi);
+    g_free(se->virtio_dev->qi);
     pthread_rwlock_destroy(&se->virtio_dev->vu_dispatch_rwlock);
-    free(se->virtio_dev);
+    g_free(se->virtio_dev);
     se->virtio_dev = NULL;
 }
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index fc7e1b1e8e..5c475a30af 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -399,7 +399,7 @@  static void lo_map_init(struct lo_map *map)
 
 static void lo_map_destroy(struct lo_map *map)
 {
-    free(map->elems);
+    g_free(map->elems);
 }
 
 static int lo_map_grow(struct lo_map *map, size_t new_nelems)
@@ -411,7 +411,7 @@  static int lo_map_grow(struct lo_map *map, size_t new_nelems)
         return 1;
     }
 
-    new_elems = realloc(map->elems, sizeof(map->elems[0]) * new_nelems);
+    new_elems = g_realloc_n(map->elems, new_nelems, sizeof(map->elems[0]));
     if (!new_elems) {
         return 0;
     }
@@ -549,7 +549,7 @@  static void lo_inode_put(struct lo_data *lo, struct lo_inode **inodep)
 
     if (g_atomic_int_dec_and_test(&inode->refcount)) {
         close(inode->fd);
-        free(inode);
+        g_free(inode);
     }
 }
 
@@ -904,7 +904,7 @@  static void posix_locks_value_destroy(gpointer data)
      * closing this fd should release all OFD locks.
      */
     close(plock->fd);
-    free(plock);
+    g_free(plock);
 }
 
 static int do_statx(struct lo_data *lo, int dirfd, const char *pathname,
@@ -1020,7 +1020,7 @@  static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
     if (inode) {
         close(newfd);
     } else {
-        inode = calloc(1, sizeof(struct lo_inode));
+        inode = g_try_new0(struct lo_inode, 1);
         if (!inode) {
             goto out_err;
         }
@@ -1532,7 +1532,7 @@  static void lo_dirp_put(struct lo_dirp **dp)
 
     if (g_atomic_int_dec_and_test(&d->refcount)) {
         closedir(d->dp);
-        free(d);
+        g_free(d);
     }
 }
 
@@ -1564,7 +1564,7 @@  static void lo_opendir(fuse_req_t req, fuse_ino_t ino,
     int fd;
     ssize_t fh;
 
-    d = calloc(1, sizeof(struct lo_dirp));
+    d = g_try_new0(struct lo_dirp, 1);
     if (d == NULL) {
         goto out_err;
     }
@@ -1606,7 +1606,7 @@  out_err:
         } else if (fd != -1) {
             close(fd);
         }
-        free(d);
+        g_free(d);
     }
     fuse_reply_err(req, error);
 }
@@ -1633,7 +1633,7 @@  static void lo_do_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
     }
 
     err = ENOMEM;
-    buf = calloc(1, size);
+    buf = g_try_malloc0(size);
     if (!buf) {
         goto error;
     }
@@ -1719,7 +1719,7 @@  error:
     } else {
         fuse_reply_buf(req, buf, size - rem);
     }
-    free(buf);
+    g_free(buf);
 }
 
 static void lo_readdir(fuse_req_t req, fuse_ino_t ino, size_t size,
@@ -1943,7 +1943,7 @@  static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo,
         return plock;
     }
 
-    plock = malloc(sizeof(struct lo_inode_plock));
+    plock = g_try_new(struct lo_inode_plock, 1);
     if (!plock) {
         *err = ENOMEM;
         return NULL;
@@ -1954,7 +1954,7 @@  static struct lo_inode_plock *lookup_create_plock_ctx(struct lo_data *lo,
     fd = lo_inode_open(lo, inode, O_RDWR);
     if (fd < 0) {
         *err = -fd;
-        free(plock);
+        g_free(plock);
         return NULL;
     }
 
@@ -2731,7 +2731,7 @@  static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
              ino, name, size);
 
     if (size) {
-        value = malloc(size);
+        value = g_try_malloc(size);
         if (!value) {
             goto out_err;
         }
@@ -2770,7 +2770,7 @@  static void lo_getxattr(fuse_req_t req, fuse_ino_t ino, const char *in_name,
         fuse_reply_xattr(req, ret);
     }
 out_free:
-    free(value);
+    g_free(value);
 
     if (fd >= 0) {
         close(fd);
@@ -2812,7 +2812,7 @@  static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
              size);
 
     if (size) {
-        value = malloc(size);
+        value = g_try_malloc(size);
         if (!value) {
             goto out_err;
         }
@@ -2897,7 +2897,7 @@  static void lo_listxattr(fuse_req_t req, fuse_ino_t ino, size_t size)
         fuse_reply_xattr(req, ret);
     }
 out_free:
-    free(value);
+    g_free(value);
 
     if (fd >= 0) {
         close(fd);