All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk()
  2021-08-17 13:52 [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck
@ 2021-08-17 12:38 ` Christian Schoenebeck
  2021-08-20 10:35   ` Greg Kurz
  2021-08-17 13:46 ` [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible Christian Schoenebeck
  2021-08-20 14:30 ` [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck
  2 siblings, 1 reply; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-17 12:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

The v9fs_walk() function resolves all client submitted path nodes to the
local 'pathes' array. Using a separate string scalar variable 'path'
inside the background worker thread loop and copying that local 'path'
string scalar variable subsequently to the 'pathes' array (at the end of
each loop iteration) is not necessary.

Instead simply resolve each path directly to the 'pathes' array and
don't use the string scalar variable 'path' inside the fs worker thread
loop at all.

The only advantage of the 'path' scalar was that in case of an error
the respective 'pathes' element would not be filled. Right now this is
not an issue as the v9fs_walk() function returns as soon as any error
occurs.

Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 2815257f42..4d642ab12a 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1787,7 +1787,8 @@ static void coroutine_fn v9fs_walk(void *opaque)
                 strcmp("..", wnames[name_idx].data))
             {
                 err = s->ops->name_to_path(&s->ctx, &dpath,
-                                        wnames[name_idx].data, &path);
+                                           wnames[name_idx].data,
+                                           &pathes[name_idx]);
                 if (err < 0) {
                     err = -errno;
                     break;
@@ -1796,14 +1797,13 @@ static void coroutine_fn v9fs_walk(void *opaque)
                     err = -EINTR;
                     break;
                 }
-                err = s->ops->lstat(&s->ctx, &path, &stbuf);
+                err = s->ops->lstat(&s->ctx, &pathes[name_idx], &stbuf);
                 if (err < 0) {
                     err = -errno;
                     break;
                 }
                 stbufs[name_idx] = stbuf;
-                v9fs_path_copy(&dpath, &path);
-                v9fs_path_copy(&pathes[name_idx], &path);
+                v9fs_path_copy(&dpath, &pathes[name_idx]);
             }
         }
     });
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible
  2021-08-17 13:52 [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck
  2021-08-17 12:38 ` [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk() Christian Schoenebeck
@ 2021-08-17 13:46 ` Christian Schoenebeck
  2021-08-17 14:10   ` Philippe Mathieu-Daudé
  2021-08-20 10:40   ` Greg Kurz
  2021-08-20 14:30 ` [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck
  2 siblings, 2 replies; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-17 13:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

Suggested-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 4d642ab12a..c857b31321 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1703,11 +1703,12 @@ static bool same_stat_id(const struct stat *a, const struct stat *b)
 static void coroutine_fn v9fs_walk(void *opaque)
 {
     int name_idx;
-    V9fsQID *qids = NULL;
+    g_autofree V9fsQID *qids = NULL;
     int i, err = 0;
     V9fsPath dpath, path, *pathes = NULL;
     uint16_t nwnames;
-    struct stat stbuf, fidst, *stbufs = NULL;
+    struct stat stbuf, fidst;
+    g_autofree struct stat *stbufs = NULL;
     size_t offset = 7;
     int32_t fid, newfid;
     V9fsString *wnames = NULL;
@@ -1872,8 +1873,6 @@ out_nofid:
             v9fs_path_free(&pathes[name_idx]);
         }
         g_free(wnames);
-        g_free(qids);
-        g_free(stbufs);
         g_free(pathes);
     }
 }
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 0/2] 9pfs: v9fs_walk() cleanup
@ 2021-08-17 13:52 Christian Schoenebeck
  2021-08-17 12:38 ` [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk() Christian Schoenebeck
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-17 13:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

Few cleanup patches for function v9fs_walk() as discussed last month.

In patch 2 array variables 'wnames' and 'pathes' are omitted because they
contain dynamically allocated memory per array element which need to be
freed individually before freeing the array.

Christian Schoenebeck (2):
  hw/9pfs: avoid 'path' copy in v9fs_walk()
  hw/9pfs: use g_autofree in v9fs_walk() where possible

 hw/9pfs/9p.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

-- 
2.20.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible
  2021-08-17 13:46 ` [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible Christian Schoenebeck
@ 2021-08-17 14:10   ` Philippe Mathieu-Daudé
  2021-08-20 10:40   ` Greg Kurz
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-17 14:10 UTC (permalink / raw)
  To: Christian Schoenebeck, qemu-devel; +Cc: Greg Kurz

On 8/17/21 3:46 PM, Christian Schoenebeck wrote:
> Suggested-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  hw/9pfs/9p.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk()
  2021-08-17 12:38 ` [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk() Christian Schoenebeck
@ 2021-08-20 10:35   ` Greg Kurz
  2021-08-20 12:19     ` Christian Schoenebeck
  0 siblings, 1 reply; 11+ messages in thread
From: Greg Kurz @ 2021-08-20 10:35 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel

On Tue, 17 Aug 2021 14:38:24 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> The v9fs_walk() function resolves all client submitted path nodes to the
> local 'pathes' array. Using a separate string scalar variable 'path'
> inside the background worker thread loop and copying that local 'path'
> string scalar variable subsequently to the 'pathes' array (at the end of
> each loop iteration) is not necessary.
> 
> Instead simply resolve each path directly to the 'pathes' array and
> don't use the string scalar variable 'path' inside the fs worker thread
> loop at all.
> 
> The only advantage of the 'path' scalar was that in case of an error
> the respective 'pathes' element would not be filled. Right now this is
> not an issue as the v9fs_walk() function returns as soon as any error
> occurs.
> 
> Suggested-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

With this change, the path variable is no longer used at all in the
first loop. I see at least an extra possible cleanup : don't set
path before the first loop since it gets reset before the second
one. Maybe we can even get rid of path all the way ? I'll have
a look.

>  hw/9pfs/9p.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 2815257f42..4d642ab12a 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1787,7 +1787,8 @@ static void coroutine_fn v9fs_walk(void *opaque)
>                  strcmp("..", wnames[name_idx].data))
>              {
>                  err = s->ops->name_to_path(&s->ctx, &dpath,
> -                                        wnames[name_idx].data, &path);
> +                                           wnames[name_idx].data,
> +                                           &pathes[name_idx]);
>                  if (err < 0) {
>                      err = -errno;
>                      break;
> @@ -1796,14 +1797,13 @@ static void coroutine_fn v9fs_walk(void *opaque)
>                      err = -EINTR;
>                      break;
>                  }
> -                err = s->ops->lstat(&s->ctx, &path, &stbuf);
> +                err = s->ops->lstat(&s->ctx, &pathes[name_idx], &stbuf);
>                  if (err < 0) {
>                      err = -errno;
>                      break;
>                  }
>                  stbufs[name_idx] = stbuf;
> -                v9fs_path_copy(&dpath, &path);
> -                v9fs_path_copy(&pathes[name_idx], &path);
> +                v9fs_path_copy(&dpath, &pathes[name_idx]);
>              }
>          }
>      });



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible
  2021-08-17 13:46 ` [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible Christian Schoenebeck
  2021-08-17 14:10   ` Philippe Mathieu-Daudé
@ 2021-08-20 10:40   ` Greg Kurz
  2021-08-20 12:23     ` Christian Schoenebeck
  1 sibling, 1 reply; 11+ messages in thread
From: Greg Kurz @ 2021-08-20 10:40 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel

On Tue, 17 Aug 2021 15:46:50 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> Suggested-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---
>  hw/9pfs/9p.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 4d642ab12a..c857b31321 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1703,11 +1703,12 @@ static bool same_stat_id(const struct stat *a, const struct stat *b)
>  static void coroutine_fn v9fs_walk(void *opaque)
>  {
>      int name_idx;
> -    V9fsQID *qids = NULL;
> +    g_autofree V9fsQID *qids = NULL;
>      int i, err = 0;
>      V9fsPath dpath, path, *pathes = NULL;
>      uint16_t nwnames;
> -    struct stat stbuf, fidst, *stbufs = NULL;
> +    struct stat stbuf, fidst;
> +    g_autofree struct stat *stbufs = NULL;
>      size_t offset = 7;
>      int32_t fid, newfid;
>      V9fsString *wnames = NULL;
> @@ -1872,8 +1873,6 @@ out_nofid:
>              v9fs_path_free(&pathes[name_idx]);
>          }
>          g_free(wnames);
> -        g_free(qids);
> -        g_free(stbufs);
>          g_free(pathes);

It seems that wnames and pathes could be converted to
g_autofree as well or I'm missing something ?

Feel free to add my R-b with or without that extra change.

Reviewed-by: Greg Kurz <groug@kaod.org>

>      }
>  }



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk()
  2021-08-20 10:35   ` Greg Kurz
@ 2021-08-20 12:19     ` Christian Schoenebeck
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-20 12:19 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

On Freitag, 20. August 2021 12:35:49 CEST Greg Kurz wrote:
> On Tue, 17 Aug 2021 14:38:24 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > The v9fs_walk() function resolves all client submitted path nodes to the
> > local 'pathes' array. Using a separate string scalar variable 'path'
> > inside the background worker thread loop and copying that local 'path'
> > string scalar variable subsequently to the 'pathes' array (at the end of
> > each loop iteration) is not necessary.
> > 
> > Instead simply resolve each path directly to the 'pathes' array and
> > don't use the string scalar variable 'path' inside the fs worker thread
> > loop at all.
> > 
> > The only advantage of the 'path' scalar was that in case of an error
> > the respective 'pathes' element would not be filled. Right now this is
> > not an issue as the v9fs_walk() function returns as soon as any error
> > occurs.
> > 
> > Suggested-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> With this change, the path variable is no longer used at all in the
> first loop. 

Correct.

> I see at least an extra possible cleanup : don't set
> path before the first loop since it gets reset before the second
> one. 

Also correct.

> Maybe we can even get rid of path all the way ? I'll have
> a look.

Yes, that's the plan.

There is still quite a bunch that can be cleaned up in that function. I just 
didn't want to start with a two-digit patch set right after the long summer 
break. ;-)

If you want to send some cleanup patches, always appreciated.

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible
  2021-08-20 10:40   ` Greg Kurz
@ 2021-08-20 12:23     ` Christian Schoenebeck
  2021-08-20 12:34       ` Greg Kurz
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-20 12:23 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

On Freitag, 20. August 2021 12:40:31 CEST Greg Kurz wrote:
> On Tue, 17 Aug 2021 15:46:50 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > Suggested-by: Greg Kurz <groug@kaod.org>
> > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > ---
> > 
> >  hw/9pfs/9p.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 4d642ab12a..c857b31321 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1703,11 +1703,12 @@ static bool same_stat_id(const struct stat *a,
> > const struct stat *b)> 
> >  static void coroutine_fn v9fs_walk(void *opaque)
> >  {
> >  
> >      int name_idx;
> > 
> > -    V9fsQID *qids = NULL;
> > +    g_autofree V9fsQID *qids = NULL;
> > 
> >      int i, err = 0;
> >      V9fsPath dpath, path, *pathes = NULL;
> >      uint16_t nwnames;
> > 
> > -    struct stat stbuf, fidst, *stbufs = NULL;
> > +    struct stat stbuf, fidst;
> > +    g_autofree struct stat *stbufs = NULL;
> > 
> >      size_t offset = 7;
> >      int32_t fid, newfid;
> >      V9fsString *wnames = NULL;
> > 
> > @@ -1872,8 +1873,6 @@ out_nofid:
> >              v9fs_path_free(&pathes[name_idx]);
> >          
> >          }
> >          g_free(wnames);
> > 
> > -        g_free(qids);
> > -        g_free(stbufs);
> > 
> >          g_free(pathes);
> 
> It seems that wnames and pathes could be converted to
> g_autofree as well or I'm missing something ?

Yeah, I mentioned that in the cover letter. Those two are omitted in this 
patch because they contain dynamically allocated memory per array element 
which need to be freed individually before freeing the array.

So those two would probably require custom cleanup handlers.

> 
> Feel free to add my R-b with or without that extra change.
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>

Thanks!

> 
> >      }
> >  
> >  }

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible
  2021-08-20 12:23     ` Christian Schoenebeck
@ 2021-08-20 12:34       ` Greg Kurz
  2021-08-20 12:49         ` Christian Schoenebeck
  0 siblings, 1 reply; 11+ messages in thread
From: Greg Kurz @ 2021-08-20 12:34 UTC (permalink / raw)
  To: Christian Schoenebeck; +Cc: qemu-devel

On Fri, 20 Aug 2021 14:23:26 +0200
Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

> On Freitag, 20. August 2021 12:40:31 CEST Greg Kurz wrote:
> > On Tue, 17 Aug 2021 15:46:50 +0200
> > 
> > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > Suggested-by: Greg Kurz <groug@kaod.org>
> > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > ---
> > > 
> > >  hw/9pfs/9p.c | 7 +++----
> > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index 4d642ab12a..c857b31321 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > > @@ -1703,11 +1703,12 @@ static bool same_stat_id(const struct stat *a,
> > > const struct stat *b)> 
> > >  static void coroutine_fn v9fs_walk(void *opaque)
> > >  {
> > >  
> > >      int name_idx;
> > > 
> > > -    V9fsQID *qids = NULL;
> > > +    g_autofree V9fsQID *qids = NULL;
> > > 
> > >      int i, err = 0;
> > >      V9fsPath dpath, path, *pathes = NULL;
> > >      uint16_t nwnames;
> > > 
> > > -    struct stat stbuf, fidst, *stbufs = NULL;
> > > +    struct stat stbuf, fidst;
> > > +    g_autofree struct stat *stbufs = NULL;
> > > 
> > >      size_t offset = 7;
> > >      int32_t fid, newfid;
> > >      V9fsString *wnames = NULL;
> > > 
> > > @@ -1872,8 +1873,6 @@ out_nofid:
> > >              v9fs_path_free(&pathes[name_idx]);
> > >          
> > >          }
> > >          g_free(wnames);
> > > 
> > > -        g_free(qids);
> > > -        g_free(stbufs);
> > > 
> > >          g_free(pathes);
> > 
> > It seems that wnames and pathes could be converted to
> > g_autofree as well or I'm missing something ?
> 
> Yeah, I mentioned that in the cover letter. Those two are omitted in this 
> patch because they contain dynamically allocated memory per array element 
> which need to be freed individually before freeing the array.
> 
> So those two would probably require custom cleanup handlers.
> 

The freeing of the individual elements is already handled in the loop above
the g_free() calls. The wnames and pathes pointers can thus be treated like
the other ones.

> > 
> > Feel free to add my R-b with or without that extra change.
> > 
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> Thanks!
> 
> > 
> > >      }
> > >  
> > >  }
> 
> Best regards,
> Christian Schoenebeck
> 
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible
  2021-08-20 12:34       ` Greg Kurz
@ 2021-08-20 12:49         ` Christian Schoenebeck
  0 siblings, 0 replies; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-20 12:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz

On Freitag, 20. August 2021 14:34:11 CEST Greg Kurz wrote:
> On Fri, 20 Aug 2021 14:23:26 +0200
> 
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Freitag, 20. August 2021 12:40:31 CEST Greg Kurz wrote:
> > > On Tue, 17 Aug 2021 15:46:50 +0200
> > > 
> > > Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > > > Suggested-by: Greg Kurz <groug@kaod.org>
> > > > Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> > > > ---
> > > > 
> > > >  hw/9pfs/9p.c | 7 +++----
> > > >  1 file changed, 3 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > > index 4d642ab12a..c857b31321 100644
> > > > --- a/hw/9pfs/9p.c
> > > > +++ b/hw/9pfs/9p.c
> > > > @@ -1703,11 +1703,12 @@ static bool same_stat_id(const struct stat *a,
> > > > const struct stat *b)>
> > > > 
> > > >  static void coroutine_fn v9fs_walk(void *opaque)
> > > >  {
> > > >  
> > > >      int name_idx;
> > > > 
> > > > -    V9fsQID *qids = NULL;
> > > > +    g_autofree V9fsQID *qids = NULL;
> > > > 
> > > >      int i, err = 0;
> > > >      V9fsPath dpath, path, *pathes = NULL;
> > > >      uint16_t nwnames;
> > > > 
> > > > -    struct stat stbuf, fidst, *stbufs = NULL;
> > > > +    struct stat stbuf, fidst;
> > > > +    g_autofree struct stat *stbufs = NULL;
> > > > 
> > > >      size_t offset = 7;
> > > >      int32_t fid, newfid;
> > > >      V9fsString *wnames = NULL;
> > > > 
> > > > @@ -1872,8 +1873,6 @@ out_nofid:
> > > >              v9fs_path_free(&pathes[name_idx]);
> > > >          
> > > >          }
> > > >          g_free(wnames);
> > > > 
> > > > -        g_free(qids);
> > > > -        g_free(stbufs);
> > > > 
> > > >          g_free(pathes);
> > > 
> > > It seems that wnames and pathes could be converted to
> > > g_autofree as well or I'm missing something ?
> > 
> > Yeah, I mentioned that in the cover letter. Those two are omitted in this
> > patch because they contain dynamically allocated memory per array element
> > which need to be freed individually before freeing the array.
> > 
> > So those two would probably require custom cleanup handlers.
> 
> The freeing of the individual elements is already handled in the loop above
> the g_free() calls. The wnames and pathes pointers can thus be treated like
> the other ones.

Yes I know. I was just considering to make that in a safer way that would 
allow simple returns in future without goto out_something; branches. But yes, 
as it is right now they could be converted in the exact same way yet.

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] 9pfs: v9fs_walk() cleanup
  2021-08-17 13:52 [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck
  2021-08-17 12:38 ` [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk() Christian Schoenebeck
  2021-08-17 13:46 ` [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible Christian Schoenebeck
@ 2021-08-20 14:30 ` Christian Schoenebeck
  2 siblings, 0 replies; 11+ messages in thread
From: Christian Schoenebeck @ 2021-08-20 14:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: Greg Kurz, Philippe Mathieu-Daudé

On Dienstag, 17. August 2021 15:52:39 CEST Christian Schoenebeck wrote:
> Few cleanup patches for function v9fs_walk() as discussed last month.
> 
> In patch 2 array variables 'wnames' and 'pathes' are omitted because they
> contain dynamically allocated memory per array element which need to be
> freed individually before freeing the array.
> 
> Christian Schoenebeck (2):
>   hw/9pfs: avoid 'path' copy in v9fs_walk()
>   hw/9pfs: use g_autofree in v9fs_walk() where possible
> 
>  hw/9pfs/9p.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)

Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next

Thanks!

Best regards,
Christian Schoenebeck




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-20 14:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 13:52 [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck
2021-08-17 12:38 ` [PATCH 1/2] hw/9pfs: avoid 'path' copy in v9fs_walk() Christian Schoenebeck
2021-08-20 10:35   ` Greg Kurz
2021-08-20 12:19     ` Christian Schoenebeck
2021-08-17 13:46 ` [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible Christian Schoenebeck
2021-08-17 14:10   ` Philippe Mathieu-Daudé
2021-08-20 10:40   ` Greg Kurz
2021-08-20 12:23     ` Christian Schoenebeck
2021-08-20 12:34       ` Greg Kurz
2021-08-20 12:49         ` Christian Schoenebeck
2021-08-20 14:30 ` [PATCH 0/2] 9pfs: v9fs_walk() cleanup Christian Schoenebeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.