From patchwork Wed Sep 14 22:14:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Sandeen X-Patchwork-Id: 9332577 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2CF9A607FD for ; Wed, 14 Sep 2016 22:14:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1D3A721BED for ; Wed, 14 Sep 2016 22:14:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 11C3A284C6; Wed, 14 Sep 2016 22:14:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from oss.sgi.com (oss.sgi.com [192.48.182.195]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3EDD821BED for ; Wed, 14 Sep 2016 22:14:49 +0000 (UTC) Received: from oss.sgi.com (localhost [IPv6:::1]) by oss.sgi.com (Postfix) with ESMTP id 64DC17CA1; Wed, 14 Sep 2016 17:14:48 -0500 (CDT) X-Original-To: xfs@oss.sgi.com Delivered-To: xfs@oss.sgi.com Received: from relay.sgi.com (relay1.corp.sgi.com [137.38.102.111]) by oss.sgi.com (Postfix) with ESMTP id 8936A7CA0 for ; Wed, 14 Sep 2016 17:14:46 -0500 (CDT) Received: from cuda.sgi.com (cuda3.sgi.com [192.48.176.15]) by relay1.corp.sgi.com (Postfix) with ESMTP id 58E488F804B for ; Wed, 14 Sep 2016 15:14:43 -0700 (PDT) X-ASG-Debug-ID: 1473891280-0bf57c1b0b7adf10001-NocioJ Received: from sandeen.net (sandeen.net [63.231.237.45]) by cuda.sgi.com with ESMTP id 8jZvHoGT6x8TkjEl for ; Wed, 14 Sep 2016 15:14:41 -0700 (PDT) X-Barracuda-Envelope-From: sandeen@sandeen.net X-Barracuda-Effective-Source-IP: sandeen.net[63.231.237.45] X-Barracuda-Apparent-Source-IP: 63.231.237.45 Received: from [10.0.0.4] (liberator [10.0.0.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by sandeen.net (Postfix) with ESMTPSA id A745E14A16; Wed, 14 Sep 2016 17:14:31 -0500 (CDT) Subject: Re: [PATCH 2/3] xfs_quota: print and path output formatting: maintain reverse compatibility To: Bill O'Donnell , linux-xfs@vger.kernel.org X-ASG-Orig-Subj: Re: [PATCH 2/3] xfs_quota: print and path output formatting: maintain reverse compatibility References: <1473866381-28975-1-git-send-email-billodo@redhat.com> <1473866381-28975-3-git-send-email-billodo@redhat.com> From: Eric Sandeen Message-ID: <0e879c60-df5f-b408-8624-5bcddb42301d@sandeen.net> Date: Wed, 14 Sep 2016 17:14:40 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1473866381-28975-3-git-send-email-billodo@redhat.com> X-Barracuda-Connect: sandeen.net[63.231.237.45] X-Barracuda-Start-Time: 1473891280 X-Barracuda-URL: https://192.48.176.15:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 6647 X-Virus-Scanned: by bsmtpd at sgi.com X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using per-user scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=2.7 tests=BSF_SC0_MISMATCH_TO X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.32901 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.00 BSF_SC0_MISMATCH_TO Envelope rcpt doesn't match header Cc: xfs@oss.sgi.com X-BeenThere: xfs@oss.sgi.com X-Mailman-Version: 2.1.14 Precedence: list List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com X-Virus-Scanned: ClamAV using ClamSMTP On 9/14/16 10:19 AM, Bill O'Donnell wrote: > This patch adjusts the formatting of the xfs_quota print and > path outputs, in order to maintain reverse compatability: > when -f flag isn't used, need to keep the output same as in > previous version. > > Signed-off-by: Bill O'Donnell > --- > quota/path.c | 67 +++++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 39 insertions(+), 28 deletions(-) > > diff --git a/quota/path.c b/quota/path.c > index aa3d33e..ed9c044 100644 > --- a/quota/path.c > +++ b/quota/path.c > @@ -36,39 +36,50 @@ printpath( > int c; > > if (index == 0) { > - printf(_("%sFilesystem Pathname\n"), > - number ? _(" ") : ""); > + if (foreign_allowed) > + printf(_("%s Filesystem Pathname\n"), > + number ? _(" ") : ""); > + else > + printf(_("%sFilesystem Pathname\n"), > + number ? _(" ") : ""); Ok, this prints the header, with differing leading whitespace depending on foreign and/or "are we printing the path number?" It works but it seems confusing to me. What about just being explicit about it, something like: + /* print header, accommodating for path nr and/or foreign flag */ if (index == 0) { + if (number) + printf(_(" ")); + if (foreign_allowed) + printf(_(" ")); + printf(_("Filesystem Pathname\n")); or maybe: if (index == 0) { + printf(_("%s%sFilesystem Pathname\n"), + number ? _(" ") : "", + foreign_allowed ? _(" ") : ""); to follow the existing code, just with another "optional" column for (F)? > } > if (number) { > printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' '); > } > - printf("%s ", (path->fs_flags & FS_FOREIGN) ? "(F)" : " "); > - printf(_("%-19s %s"), path->fs_dir, path->fs_name); > - if (path->fs_flags & FS_PROJECT_PATH) { > - prj = getprprid(path->fs_prid); > - printf(_(" (project %u"), path->fs_prid); > - if (prj) > - printf(_(", %s"), prj->pr_name); > - printf(")"); > - } else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0, > - (void *)&qstat) == 0 && qstat.qs_flags) { > - c = 0; > - printf(" ("); > - if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD) > - c = printf("uquota"); > - else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT) > - c = printf("uqnoenforce"); > - if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD) > - c = printf("%sgquota", c ? ", " : ""); > - else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT) > - c = printf("%sgqnoenforce", c ? ", " : ""); > - if (qstat.qs_flags & XFS_QUOTA_PDQ_ENFD) > - printf("%spquota", c ? ", " : ""); > - else if (qstat.qs_flags & XFS_QUOTA_PDQ_ACCT) > - printf("%spqnoenforce", c ? ", " : ""); > - printf(")"); Ok below is mostly just moving indentation under this: > + if (!((path->fs_flags & FS_FOREIGN) && !foreign_allowed)) { a big conditional, which I think means "If it's not a foreign filesystem with foreign filesystems disallowed..." Can we ever get her when that's not true? Do we ever call into this with a foreign fs when !foreign_allowed? I don't think so, because we break in the pathlist_f caller, right? and in print_f ... oh right, need to break out of print_f, too. See my latest reply to patch 1. With that, should not need to move any of this code under the conditional; it can go away and the code can remain where it's at. > + printf("%s", (path->fs_flags & FS_FOREIGN) ? "(F) " : ""); > + if (path->fs_flags & FS_FOREIGN) > + printf(_("%-19s %s"), path->fs_dir, path->fs_name); > + else if (foreign_allowed) > + printf(_(" %-19s %s"), path->fs_dir, path->fs_name); > + else > + printf(_("%-19s %s"), path->fs_dir, path->fs_name); Hm, above you've got 3 cases for 2 different printf strings. ;) This would be much simpler as: if (number) { printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' '); + if (foreign_allowed) + printf("%s", path->fs_flags & FS_FOREIGN ? "(F) " : " "); + printf(_("%-19s %s"), path->fs_dir, path->fs_name); So if we're printing numbers, print that column; if we're dealing with foreign FS's, print that column, and then print the common information in the last 2 columns. > + if (path->fs_flags & FS_PROJECT_PATH) { > + prj = getprprid(path->fs_prid); > + printf(_(" (project %u"), path->fs_prid); > + if (prj) > + printf(_(", %s"), prj->pr_name); > + printf(")"); > + } else if (xfsquotactl(XFS_GETQSTAT, path->fs_name, 0, 0, > + (void *)&qstat) == 0 && qstat.qs_flags) { > + c = 0; > + printf(" ("); > + if (qstat.qs_flags & XFS_QUOTA_UDQ_ENFD) > + c = printf("uquota"); > + else if (qstat.qs_flags & XFS_QUOTA_UDQ_ACCT) > + c = printf("uqnoenforce"); > + if (qstat.qs_flags & XFS_QUOTA_GDQ_ENFD) > + c = printf("%sgquota", c ? ", " : ""); > + else if (qstat.qs_flags & XFS_QUOTA_GDQ_ACCT) > + c = printf("%sgqnoenforce", c ? ", " : ""); > + if (qstat.qs_flags & XFS_QUOTA_PDQ_ENFD) > + printf("%spquota", c ? ", " : ""); > + else if (qstat.qs_flags & XFS_QUOTA_PDQ_ACCT) > + printf("%spqnoenforce", c ? ", " : ""); > + printf(")"); > + } > + printf("\n"); and again w/o the conditional this whole indentation move goes away, and the patch looks much simpler overall :) - (but don't take my word for it! ;) I think this works though.) diff --git a/quota/path.c b/quota/path.c index cf00fc5..622c6b5 100644 --- a/quota/path.c +++ b/quota/path.c @@ -36,14 +36,19 @@ printpath( int c; if (index == 0) { - printf(_("%sFilesystem Pathname\n"), - number ? _(" ") : ""); + printf(_("%s%sFilesystem Pathname\n"), + number ? _(" ") : "", + foreign_allowed ? _(" ") : ""); } - if (number) { + + if (number) printf(_("%c%03d%c "), braces? '[':' ', index, braces? ']':' '); - } - printf("%s ", (path->fs_flags & FS_FOREIGN) ? "(F)" : " "); + + if (foreign_allowed) + printf("%s", path->fs_flags & FS_FOREIGN ? "(F) " : " "); + printf(_("%-19s %s"), path->fs_dir, path->fs_name); + if (path->fs_flags & FS_PROJECT_PATH) { prj = getprprid(path->fs_prid); printf(_(" (project %u"), path->fs_prid);