Re: [PATCH 8/9] hw/9pfs/9p-synth: Replaced qemu_mutex_lock with QEMU_LOCK_GUARD

[Mahmoud Mandour]

Thanks for the fast review. I asked on the QEMU IRC channel 

before committing whether to put all the changes into one patch 

or split them and was instructed that it was better to split them up. 

But in any case I was open to both ways and you can decide

on the best way to go. 

On Thu, Mar 11, 2021 at 1:59 PM Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:

On Donnerstag, 11. März 2021 12:52:45 CET Greg Kurz wrote:
> On Thu, 11 Mar 2021 11:49:06 +0100
>
> Christian Schoenebeck <qemu_oss@crudebyte.com> wrote:
> > On Donnerstag, 11. März 2021 04:15:37 CET Mahmoud Mandour wrote:
> > > Replaced a call to qemu_mutex_lock and its respective call to
> > > qemu_mutex_unlock and used QEMU_LOCK_GUARD macro in their place.
> > > This simplifies the code by removing the call required to unlock
> > > and also eliminates goto paths.
> > >
> > > Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> > > ---
> > >
> > >  hw/9pfs/9p-synth.c | 12 ++++--------
> > >  1 file changed, 4 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
> > > index 7eb210ffa8..473ef914b0 100644
> > > --- a/hw/9pfs/9p-synth.c
> > > +++ b/hw/9pfs/9p-synth.c
> > > @@ -79,11 +79,11 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int
> > > mode, if (!parent) {
> > >
> > >          parent = &synth_root;
> > >     
> > >      }
> > >
> > > -    qemu_mutex_lock(&synth_mutex);
> > > +    QEMU_LOCK_GUARD(&synth_mutex);
> > >
> > >      QLIST_FOREACH(tmp, &parent->child, sibling) {
> > >     
> > >          if (!strcmp(tmp->name, name)) {
> > >         
> > >              ret = EEXIST;
> > >
> > > -            goto err_out;
> > > +            return ret;
> > >
> > >          }
> > >     
> > >      }
> > >      /* Add the name */
> > >
> > > @@ -94,8 +94,6 @@ int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int
> > > mode, node->attr, node->attr->inode);
> > >
> > >      *result = node;
> > >      ret = 0;
> > >
> > > -err_out:
> > > -    qemu_mutex_unlock(&synth_mutex);
> > >
> > >      return ret;
> > > 
> > >  }
> > >
> > > @@ -116,11 +114,11 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode
> > > *parent,
> > > int mode, parent = &synth_root;
> > >
> > >      }
> > >
> > > -    qemu_mutex_lock(&synth_mutex);
> > > +    QEMU_LOCK_GUARD(&synth_mutex);
> > >
> > >      QLIST_FOREACH(tmp, &parent->child, sibling) {
> > >     
> > >          if (!strcmp(tmp->name, name)) {
> > >         
> > >              ret = EEXIST;
> > >
> > > -            goto err_out;
> > > +            return ret;
> > >
> > >          }
> > >     
> > >      }
> > >      /* Add file type and remove write bits */
> > >
> > > @@ -136,8 +134,6 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent,
> > > int
> > > mode, pstrcpy(node->name, sizeof(node->name), name);
> > >
> > >      QLIST_INSERT_HEAD_RCU(&parent->child, node, sibling);
> > >      ret = 0;
> > >
> > > -err_out:
> > > -    qemu_mutex_unlock(&synth_mutex);
> > >
> > >      return ret;
> > > 
> > >  }
> >
> > Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> >
> > Greg, I suggest I'll push this onto my queue as you seem to be busy.
>
> This cleanup spans over multiple subsystems but I think it makes more
> sense to keep all these patches together. Let's wait for everyone to
> ack/review and then we'll decide how to merge the patches.

Sure, makes sense.