Re: [PATCH 2/2] hw/9pfs: use g_autofree in v9fs_walk() where possible

[Greg Kurz]

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
> 
>