* [PATCH] xfs_quota: refactor code to generate id from name
@ 2020-05-09 17:18 Eric Sandeen
2020-05-11 15:32 ` Christoph Hellwig
2020-05-11 19:08 ` [PATCH V2] " Eric Sandeen
0 siblings, 2 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-05-09 17:18 UTC (permalink / raw)
To: linux-xfs
There's boilerplate for setting limits and warnings, where we have
a case statement for each of the 3 quota types, and from there call
3 different functions to configure each of the 3 types, each of which
calls its own version of id to string function...
Refactor this so that the main function can call a generic id to string
conversion routine, and then call a common action. This save a lot of
LOC.
I was looking at allowing xfs to bump out individual grace periods like
setquota can do, and this refactoring allows us to add new actions like
that without copyingall the boilerplate again.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
edit.c | 196 ++++++++++++++++-----------------------------------------
1 file changed, 51 insertions(+), 145 deletions(-)
diff --git a/quota/edit.c b/quota/edit.c
index f9938b8a..70c0969f 100644
--- a/quota/edit.c
+++ b/quota/edit.c
@@ -101,6 +101,42 @@ warn_help(void)
"\n"));
}
+static uint32_t
+id_from_string(
+ char *name,
+ int type)
+{
+ uint32_t id = -1;
+
+ switch (type) {
+ case XFS_USER_QUOTA:
+ id = uid_from_string(name);
+ if (id == -1)
+ fprintf(stderr, _("%s: invalid user name: %s\n"),
+ progname, name);
+ break;
+ case XFS_GROUP_QUOTA:
+ id = gid_from_string(name);
+ if (id == -1)
+ fprintf(stderr, _("%s: invalid group name: %s\n"),
+ progname, name);
+ break;
+ case XFS_PROJ_QUOTA:
+ id = prid_from_string(name);
+ if (id == -1)
+ fprintf(stderr, _("%s: invalid project name: %s\n"),
+ progname, name);
+ break;
+ default:
+ ASSERT(0);
+ break;
+ }
+
+ if (id == -1)
+ exitcode = 1;
+ return id;
+}
+
static void
set_limits(
uint32_t id,
@@ -135,75 +171,6 @@ set_limits(
}
}
-static void
-set_user_limits(
- char *name,
- uint type,
- uint mask,
- uint64_t *bsoft,
- uint64_t *bhard,
- uint64_t *isoft,
- uint64_t *ihard,
- uint64_t *rtbsoft,
- uint64_t *rtbhard)
-{
- uid_t uid = uid_from_string(name);
-
- if (uid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid user name: %s\n"),
- progname, name);
- } else
- set_limits(uid, type, mask, fs_path->fs_name,
- bsoft, bhard, isoft, ihard, rtbsoft, rtbhard);
-}
-
-static void
-set_group_limits(
- char *name,
- uint type,
- uint mask,
- uint64_t *bsoft,
- uint64_t *bhard,
- uint64_t *isoft,
- uint64_t *ihard,
- uint64_t *rtbsoft,
- uint64_t *rtbhard)
-{
- gid_t gid = gid_from_string(name);
-
- if (gid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid group name: %s\n"),
- progname, name);
- } else
- set_limits(gid, type, mask, fs_path->fs_name,
- bsoft, bhard, isoft, ihard, rtbsoft, rtbhard);
-}
-
-static void
-set_project_limits(
- char *name,
- uint type,
- uint mask,
- uint64_t *bsoft,
- uint64_t *bhard,
- uint64_t *isoft,
- uint64_t *ihard,
- uint64_t *rtbsoft,
- uint64_t *rtbhard)
-{
- prid_t prid = prid_from_string(name);
-
- if (prid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid project name: %s\n"),
- progname, name);
- } else
- set_limits(prid, type, mask, fs_path->fs_name,
- bsoft, bhard, isoft, ihard, rtbsoft, rtbhard);
-}
-
/* extract number of blocks from an ascii string */
static int
extractb(
@@ -258,6 +225,7 @@ limit_f(
char **argv)
{
char *name;
+ uint32_t id;
uint64_t bsoft, bhard, isoft, ihard, rtbsoft, rtbhard;
int c, type = 0, mask = 0, flags = 0;
uint bsize, ssize, endoptions;
@@ -339,20 +307,13 @@ limit_f(
return command_usage(&limit_cmd);
}
- switch (type) {
- case XFS_USER_QUOTA:
- set_user_limits(name, type, mask,
- &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
- break;
- case XFS_GROUP_QUOTA:
- set_group_limits(name, type, mask,
- &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
- break;
- case XFS_PROJ_QUOTA:
- set_project_limits(name, type, mask,
- &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
- break;
- }
+
+ id = id_from_string(name, type);
+ if (id >= 0)
+ set_limits(id, type, mask, fs_path->fs_name,
+ &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
+ else
+ exitcode = -1;
return 0;
}
@@ -561,63 +522,13 @@ set_warnings(
}
}
-static void
-set_user_warnings(
- char *name,
- uint type,
- uint mask,
- uint value)
-{
- uid_t uid = uid_from_string(name);
-
- if (uid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid user name: %s\n"),
- progname, name);
- } else
- set_warnings(uid, type, mask, fs_path->fs_name, value);
-}
-
-static void
-set_group_warnings(
- char *name,
- uint type,
- uint mask,
- uint value)
-{
- gid_t gid = gid_from_string(name);
-
- if (gid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid group name: %s\n"),
- progname, name);
- } else
- set_warnings(gid, type, mask, fs_path->fs_name, value);
-}
-
-static void
-set_project_warnings(
- char *name,
- uint type,
- uint mask,
- uint value)
-{
- prid_t prid = prid_from_string(name);
-
- if (prid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid project name: %s\n"),
- progname, name);
- } else
- set_warnings(prid, type, mask, fs_path->fs_name, value);
-}
-
static int
warn_f(
int argc,
char **argv)
{
char *name;
+ uint32_t id;
uint value;
int c, flags = 0, type = 0, mask = 0;
@@ -675,17 +586,12 @@ warn_f(
return command_usage(&warn_cmd);
}
- switch (type) {
- case XFS_USER_QUOTA:
- set_user_warnings(name, type, mask, value);
- break;
- case XFS_GROUP_QUOTA:
- set_group_warnings(name, type, mask, value);
- break;
- case XFS_PROJ_QUOTA:
- set_project_warnings(name, type, mask, value);
- break;
- }
+ id = id_from_string(name, type);
+ if (id >= 0)
+ set_warnings(id, type, mask, fs_path->fs_name, value);
+ else
+ exitcode = -1;
+
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_quota: refactor code to generate id from name
2020-05-09 17:18 [PATCH] xfs_quota: refactor code to generate id from name Eric Sandeen
@ 2020-05-11 15:32 ` Christoph Hellwig
2020-05-11 16:00 ` Eric Sandeen
2020-05-11 19:08 ` [PATCH V2] " Eric Sandeen
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-05-11 15:32 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Sat, May 09, 2020 at 12:18:42PM -0500, Eric Sandeen wrote:
> There's boilerplate for setting limits and warnings, where we have
> a case statement for each of the 3 quota types, and from there call
> 3 different functions to configure each of the 3 types, each of which
> calls its own version of id to string function...
>
> Refactor this so that the main function can call a generic id to string
> conversion routine, and then call a common action. This save a lot of
> LOC.
>
> I was looking at allowing xfs to bump out individual grace periods like
> setquota can do, and this refactoring allows us to add new actions like
> that without copyingall the boilerplate again.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> edit.c | 196 ++++++++++++++++-----------------------------------------
> 1 file changed, 51 insertions(+), 145 deletions(-)
>
> diff --git a/quota/edit.c b/quota/edit.c
> index f9938b8a..70c0969f 100644
> --- a/quota/edit.c
> +++ b/quota/edit.c
> @@ -101,6 +101,42 @@ warn_help(void)
> "\n"));
> }
>
> +static uint32_t
> +id_from_string(
> + char *name,
> + int type)
> +{
> + uint32_t id = -1;
> +
> + switch (type) {
> + case XFS_USER_QUOTA:
> + id = uid_from_string(name);
> + if (id == -1)
> + fprintf(stderr, _("%s: invalid user name: %s\n"),
> + progname, name);
> + break;
> + case XFS_GROUP_QUOTA:
> + id = gid_from_string(name);
> + if (id == -1)
> + fprintf(stderr, _("%s: invalid group name: %s\n"),
> + progname, name);
> + break;
> + case XFS_PROJ_QUOTA:
> + id = prid_from_string(name);
> + if (id == -1)
> + fprintf(stderr, _("%s: invalid project name: %s\n"),
> + progname, name);
> + break;
> + default:
> + ASSERT(0);
> + break;
> + }
> +
> + if (id == -1)
> + exitcode = 1;
> + return id;
What about de-duplicating the error printk as well?
static uint32_t
id_from_string(
char *name,
int type)
{
uint32_t id = -1;
const char *type = "invalid";
switch (type) {
case XFS_USER_QUOTA:
type = "user";
id = uid_from_string(name);
break;
case XFS_GROUP_QUOTA:
type = "group";
id = gid_from_string(name);
break;
case XFS_PROJ_QUOTA:
type = "project";
id = prid_from_string(name);
break;
default:
ASSERT(0);
break;
}
if (id == -1) {
fprintf(stderr, _("%s: invalid %s name: %s\n"),
type, progname, name);
exitcode = 1;
}
return id;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs_quota: refactor code to generate id from name
2020-05-11 15:32 ` Christoph Hellwig
@ 2020-05-11 16:00 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-05-11 16:00 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On 5/11/20 10:32 AM, Christoph Hellwig wrote:
> On Sat, May 09, 2020 at 12:18:42PM -0500, Eric Sandeen wrote:
...
>> @@ -101,6 +101,42 @@ warn_help(void)
>> "\n"));
>> }
>>
>> +static uint32_t
>> +id_from_string(
>> + char *name,
>> + int type)
>> +{
>> + uint32_t id = -1;
>> +
>> + switch (type) {
>> + case XFS_USER_QUOTA:
>> + id = uid_from_string(name);
>> + if (id == -1)
>> + fprintf(stderr, _("%s: invalid user name: %s\n"),
>> + progname, name);
>> + break;
>> + case XFS_GROUP_QUOTA:
>> + id = gid_from_string(name);
>> + if (id == -1)
>> + fprintf(stderr, _("%s: invalid group name: %s\n"),
>> + progname, name);
>> + break;
>> + case XFS_PROJ_QUOTA:
>> + id = prid_from_string(name);
>> + if (id == -1)
>> + fprintf(stderr, _("%s: invalid project name: %s\n"),
>> + progname, name);
>> + break;
>> + default:
>> + ASSERT(0);
>> + break;
>> + }
>> +
>> + if (id == -1)
>> + exitcode = 1;
>> + return id;
>
> What about de-duplicating the error printk as well?
Oh yeah, good idea, I'll do that and send V2.
for some reason I always forget about handling things like this in
this way. :)
Thanks,
-Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2] xfs_quota: refactor code to generate id from name
2020-05-09 17:18 [PATCH] xfs_quota: refactor code to generate id from name Eric Sandeen
2020-05-11 15:32 ` Christoph Hellwig
@ 2020-05-11 19:08 ` Eric Sandeen
2020-05-12 8:09 ` Christoph Hellwig
1 sibling, 1 reply; 6+ messages in thread
From: Eric Sandeen @ 2020-05-11 19:08 UTC (permalink / raw)
To: linux-xfs; +Cc: Christoph Hellwig
There's boilerplate for setting limits and warnings, where we have
a case statement for each of the 3 quota types, and from there call
3 different functions to configure each of the 3 types, each of which
calls its own version of id to string function...
Refactor this so that the main function can call a generic id to string
conversion routine, and then call a common action. This save a lot of
LOC.
I was looking at allowing xfs to bump out individual grace periods like
setquota can do, and this refactoring allows us to add new actions like
that without copying all the boilerplate again.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
V2: dedupe printfs per hch's suggestion
edit.c | 194 +++++++++++--------------------------
1 file changed, 49 insertions(+), 145 deletions(-)
diff --git a/quota/edit.c b/quota/edit.c
index f9938b8a..c40985e2 100644
--- a/quota/edit.c
+++ b/quota/edit.c
@@ -101,6 +101,40 @@ warn_help(void)
"\n"));
}
+static uint32_t
+id_from_string(
+ char *name,
+ int type)
+{
+ uint32_t id = -1;
+ const char *type_name = "unknown type";
+
+ switch (type) {
+ case XFS_USER_QUOTA:
+ type_name = "user";
+ id = uid_from_string(name);
+ break;
+ case XFS_GROUP_QUOTA:
+ type_name = "group";
+ id = gid_from_string(name);
+ break;
+ case XFS_PROJ_QUOTA:
+ type_name = "project";
+ id = prid_from_string(name);
+ break;
+ default:
+ ASSERT(0);
+ break;
+ }
+
+ if (id == -1) {
+ fprintf(stderr, _("%s: invalid %s name: %s\n"),
+ type_name, progname, name);
+ exitcode = 1;
+ }
+ return id;
+}
+
static void
set_limits(
uint32_t id,
@@ -135,75 +169,6 @@ set_limits(
}
}
-static void
-set_user_limits(
- char *name,
- uint type,
- uint mask,
- uint64_t *bsoft,
- uint64_t *bhard,
- uint64_t *isoft,
- uint64_t *ihard,
- uint64_t *rtbsoft,
- uint64_t *rtbhard)
-{
- uid_t uid = uid_from_string(name);
-
- if (uid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid user name: %s\n"),
- progname, name);
- } else
- set_limits(uid, type, mask, fs_path->fs_name,
- bsoft, bhard, isoft, ihard, rtbsoft, rtbhard);
-}
-
-static void
-set_group_limits(
- char *name,
- uint type,
- uint mask,
- uint64_t *bsoft,
- uint64_t *bhard,
- uint64_t *isoft,
- uint64_t *ihard,
- uint64_t *rtbsoft,
- uint64_t *rtbhard)
-{
- gid_t gid = gid_from_string(name);
-
- if (gid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid group name: %s\n"),
- progname, name);
- } else
- set_limits(gid, type, mask, fs_path->fs_name,
- bsoft, bhard, isoft, ihard, rtbsoft, rtbhard);
-}
-
-static void
-set_project_limits(
- char *name,
- uint type,
- uint mask,
- uint64_t *bsoft,
- uint64_t *bhard,
- uint64_t *isoft,
- uint64_t *ihard,
- uint64_t *rtbsoft,
- uint64_t *rtbhard)
-{
- prid_t prid = prid_from_string(name);
-
- if (prid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid project name: %s\n"),
- progname, name);
- } else
- set_limits(prid, type, mask, fs_path->fs_name,
- bsoft, bhard, isoft, ihard, rtbsoft, rtbhard);
-}
-
/* extract number of blocks from an ascii string */
static int
extractb(
@@ -258,6 +223,7 @@ limit_f(
char **argv)
{
char *name;
+ uint32_t id;
uint64_t bsoft, bhard, isoft, ihard, rtbsoft, rtbhard;
int c, type = 0, mask = 0, flags = 0;
uint bsize, ssize, endoptions;
@@ -339,20 +305,13 @@ limit_f(
return command_usage(&limit_cmd);
}
- switch (type) {
- case XFS_USER_QUOTA:
- set_user_limits(name, type, mask,
- &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
- break;
- case XFS_GROUP_QUOTA:
- set_group_limits(name, type, mask,
- &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
- break;
- case XFS_PROJ_QUOTA:
- set_project_limits(name, type, mask,
- &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
- break;
- }
+
+ id = id_from_string(name, type);
+ if (id >= 0)
+ set_limits(id, type, mask, fs_path->fs_name,
+ &bsoft, &bhard, &isoft, &ihard, &rtbsoft, &rtbhard);
+ else
+ exitcode = -1;
return 0;
}
@@ -561,63 +520,13 @@ set_warnings(
}
}
-static void
-set_user_warnings(
- char *name,
- uint type,
- uint mask,
- uint value)
-{
- uid_t uid = uid_from_string(name);
-
- if (uid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid user name: %s\n"),
- progname, name);
- } else
- set_warnings(uid, type, mask, fs_path->fs_name, value);
-}
-
-static void
-set_group_warnings(
- char *name,
- uint type,
- uint mask,
- uint value)
-{
- gid_t gid = gid_from_string(name);
-
- if (gid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid group name: %s\n"),
- progname, name);
- } else
- set_warnings(gid, type, mask, fs_path->fs_name, value);
-}
-
-static void
-set_project_warnings(
- char *name,
- uint type,
- uint mask,
- uint value)
-{
- prid_t prid = prid_from_string(name);
-
- if (prid == -1) {
- exitcode = 1;
- fprintf(stderr, _("%s: invalid project name: %s\n"),
- progname, name);
- } else
- set_warnings(prid, type, mask, fs_path->fs_name, value);
-}
-
static int
warn_f(
int argc,
char **argv)
{
char *name;
+ uint32_t id;
uint value;
int c, flags = 0, type = 0, mask = 0;
@@ -675,17 +584,12 @@ warn_f(
return command_usage(&warn_cmd);
}
- switch (type) {
- case XFS_USER_QUOTA:
- set_user_warnings(name, type, mask, value);
- break;
- case XFS_GROUP_QUOTA:
- set_group_warnings(name, type, mask, value);
- break;
- case XFS_PROJ_QUOTA:
- set_project_warnings(name, type, mask, value);
- break;
- }
+ id = id_from_string(name, type);
+ if (id >= 0)
+ set_warnings(id, type, mask, fs_path->fs_name, value);
+ else
+ exitcode = -1;
+
return 0;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2] xfs_quota: refactor code to generate id from name
2020-05-11 19:08 ` [PATCH V2] " Eric Sandeen
@ 2020-05-12 8:09 ` Christoph Hellwig
2020-05-12 15:20 ` Eric Sandeen
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2020-05-12 8:09 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs, Christoph Hellwig
> +static uint32_t
> +id_from_string(
> + char *name,
> + int type)
> +{
> + uint32_t id = -1;
> + const char *type_name = "unknown type";
Any reason to align the arguments different from the local variables?
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2] xfs_quota: refactor code to generate id from name
2020-05-12 8:09 ` Christoph Hellwig
@ 2020-05-12 15:20 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2020-05-12 15:20 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On 5/12/20 3:09 AM, Christoph Hellwig wrote:
>> +static uint32_t
>> +id_from_string(
>> + char *name,
>> + int type)
>> +{
>> + uint32_t id = -1;
>> + const char *type_name = "unknown type";
>
> Any reason to align the arguments different from the local variables?
nah I'll fix that on the way in
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-12 15:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09 17:18 [PATCH] xfs_quota: refactor code to generate id from name Eric Sandeen
2020-05-11 15:32 ` Christoph Hellwig
2020-05-11 16:00 ` Eric Sandeen
2020-05-11 19:08 ` [PATCH V2] " Eric Sandeen
2020-05-12 8:09 ` Christoph Hellwig
2020-05-12 15:20 ` Eric Sandeen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.