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

[Christian Schoenebeck]

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