MIME-Version: 1.0
References: <20190326141109.16844-1-kdsouza@redhat.com> <20190409205011.GD29099@fieldses.org>
In-Reply-To: <20190409205011.GD29099@fieldses.org>
From:   Kenneth Dsouza <kdsouza@redhat.com>
Date:   Wed, 10 Apr 2019 14:58:11 +0530
Message-ID: <CAA_-hQJhCgSVqRSiPzHPqhOoxMM9Bi=F3x=Hc0v6NBe=NeXf1A@mail.gmail.com>
Subject: Re: [PATCH] nfs4_setfacl: Skip comment field while reading ACE(s).
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Thanks for taking care of this.

On Wed, Apr 10, 2019 at 2:20 AM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> Applied, with some fixes:
>
> On Tue, Mar 26, 2019 at 07:41:09PM +0530, Kenneth D'souza wrote:
> > With commit 6630629bb661a7f48fb9856f7fd9616ce1499efa an additional field for filename
> > was added due to which nfs4_setfacl failed to handle comments while reading ACE(s)
> > from nfs4_getfacl output.
> > This patch resolves the issue by skipping comment header.
> >
> >  With fix:
> >
> >  $ nfs4_setfacl --test -s "$(nfs4_getfacl  file1)" file2
> >  Skipping comment # file: file1
>
> Just skip it silently, there's not need to print anything.
>
> >  ## Test mode only - the resulting ACL for "/test/file2":
> >  A::OWNER@:rwatTcCy
> >  A:g:GROUP@:rtcy
> >  A::EVERYONE@:rtcy
> >
> >  Without fix:
> >
> >  $ nfs4_setfacl --test -s "$(nfs4_getfacl  file1)" file2
> >  Failed while inserting ACE(s).
> >
> > Signed-off-by: Kenneth D'souza <kdsouza@redhat.com>
> > ---
> >  libnfs4acl/nfs4_insert_string_aces.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/libnfs4acl/nfs4_insert_string_aces.c b/libnfs4acl/nfs4_insert_string_aces.c
> > index 5a482d5..50b7bbf 100644
> > --- a/libnfs4acl/nfs4_insert_string_aces.c
> > +++ b/libnfs4acl/nfs4_insert_string_aces.c
> > @@ -45,21 +45,25 @@ int nfs4_insert_string_aces(struct nfs4_acl *acl, const char *acl_spec, unsigned
> >       if ((s = sp = strdup(acl_spec)) == NULL)
> >               goto out_failed;
> >
> > +
> >       while ((ssp = strsep(&sp, ",\t\n\r")) != NULL) {
> >               if (!strlen(ssp))
> >                       continue;
> >
> > -             if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
> > -                     goto out_failed;
> > +             if(*ssp == '#')
> > +                     printf("Skipping comment %s\n", ssp);
>
> So let's just do a "continue" here, as above.  Also notice we usually
> leave a space after "if".
>
> --b.
>
> > +             else {
> > +                     if ((ace = nfs4_ace_from_string(ssp, acl->is_directory)) == NULL)
> > +                             goto out_failed;
> >
> > -             if (nfs4_insert_ace_at(acl, ace, index++)) {
> > -                     free(ace);
> > -                     goto out_failed;
> > +                     if (nfs4_insert_ace_at(acl, ace, index++)) {
> > +                             free(ace);
> > +                             goto out_failed;
> > +                     }
> >               }
> >       }
> >       if (acl->naces == 0)
> >               goto out_failed;
> > -
> >  out:
> >       if (s)
> >               free(s);
> > --
> > 2.20.1