MIME-Version: 1.0
References: <20190326135740.16558-1-kdsouza@redhat.com> <20190409203427.GC29099@fieldses.org>
 <CAA_-hQ+fhGCiue4aGZiDtdHy1_grHCnSPa8q8-bMdnxE-PP=AA@mail.gmail.com> <20190410132607.GA2714@fieldses.org>
In-Reply-To: <20190410132607.GA2714@fieldses.org>
From:   Kenneth Dsouza <kdsouza@redhat.com>
Date:   Wed, 10 Apr 2019 19:03:54 +0530
Message-ID: <CAA_-hQKa0Am2T1xDw0RFOVg5FB__DJUDfFfsKZoeVns3QwD5DQ@mail.gmail.com>
Subject: Re: [PATCH] nfs4_getfacl: Add new option -c/--omit-header to not
 display comment header.
To:     "J. Bruce Fields" <bfields@fieldses.org>
Cc:     linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Thanks! Should we also add the below entries similar to getfacl?

$ nfs4_getfacl /test/
# file: /test/
# owner: test
# group: test
A::OWNER@:rwaDxtTcCy
A::GROUP@:rwaDxtcy
A::EVERYONE@:rwaDxtcy

On Wed, Apr 10, 2019 at 6:56 PM J. Bruce Fields <bfields@fieldses.org> wrote:
>
> On Wed, Apr 10, 2019 at 03:07:11PM +0530, Kenneth Dsouza wrote:
> > On Wed, Apr 10, 2019 at 2:04 AM J. Bruce Fields <bfields@fieldses.org> wrote:
> > >
> > > Thanks, applying with some fixes:
> > >
> > > On Tue, Mar 26, 2019 at 07:27:40PM +0530, Kenneth D'souza wrote:
> > > > With this patch the filename will not be printed.
> > > >
> > > >     $ nfs4_getfacl /test/
> > > >     # file: /test/
> > > >     A::OWNER@:rwaDxtTcCy
> > > >     A::GROUP@:rwaDxtcy
> > > >     A::EVERYONE@:rwaDxtcy
> > > >
> > > >     $ nfs4_getfacl -c /test/
> > > >
> > > >     A::OWNER@:rwaDxtTcCy
> > > >     A::GROUP@:rwaDxtcy
> > > >     A::EVERYONE@:rwaDxtcy
> > >
> > > I don't see any reason for that blank line, I've removed it.
> > >
> > I added the blank line so we print acl for each file on a new line
> > when -c is used.
>
> OK, it does make sense to have a blank line between multiple ACLs.
>
> Again I'd rather adopt the same behavior has getfacl/setfacl whenever we
> can.  Looks like getfacl just appends a blank line regardless of whether
> -c is used or whether the are multiple ACLs:
>
>         [bfields@patate ~]$ getfacl .
>         # file: .
>         # owner: bfields
>         # group: bfields
>         user::rwx
>         group::--x
>         other::--x
>
>         [bfields@patate ~]$
>
> I've added that to print_acl_from_path().
>
> --b.
>
> > # nfs4_getfacl -Rc /test
> >
> > A::OWNER@:rwaDxtTcCy
> > A::GROUP@:rwaDxtcy
> > A::EVERYONE@:rwaDxtcy
> >
> > A::OWNER@:rwatTcCy
> > A::GROUP@:tcy
> > A::EVERYONE@:tcy
> >
> > A::OWNER@:rwaDxtTcCy
> > A::GROUP@:rxtcy
> > A::EVERYONE@:rxtcy
> >
> >
> > > > @@ -115,7 +119,10 @@ static void print_acl_from_path(const char *fpath)
> > > >       struct nfs4_acl *acl;
> > > >       acl = nfs4_acl_for_path(fpath);
> > > >       if (acl != NULL) {
> > > > +             if(ignore_comment == 0)
> > > >               printf("\n# file: %s\n", fpath);
> > >
> > > This needs to be indented.
> > >
> > > > +             else
> > > > +                     printf("\n");
> > >
> > > I've removed the "else" clause.
> > >
> > > And fixed a couple minor whitespace issues.
> > >
> > > --b.
> > >
> > > >               nfs4_print_acl(stdout, acl);
> > > >               nfs4_free_acl(acl);
> > > >       }
> > > > @@ -125,7 +132,7 @@ static void usage(int label)
> > > >  {
> > > >       if (label)
> > > >               fprintf(stderr, "%s %s -- get NFSv4 file or directory access control lists.\n", execname, VERSION);
> > > > -     fprintf(stderr, "Usage: %s [-R] file ...\n  -H, --more-help\tdisplay ACL format information\n  -?, -h, --help\tdisplay this help text\n  -R --recursive\trecurse into subdirectories\n", execname);
> > > > +     fprintf(stderr, "Usage: %s [-R] file ...\n  -H, --more-help\tdisplay ACL format information\n  -?, -h, --help\tdisplay this help text\n  -R --recursive\trecurse into subdirectories\n  -c, --omit-header\tDo not display the comment header (Do not print filename)\n", execname);
> > > >  }
> > > >
> > > >  static void more_help()
> > > > --
> > > > 2.20.1