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 <billodo@xxxxxxxxxx>
> ---
> 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 :) -
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);
(but don't take my word for it! ;) I think this works though.)
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
