[v1] Various cleanups

[libvirt PATCH 11/14] storage: createFileDir: use less ternary operators

Posted by Ján Tomko 5 years, 4 months ago

Introduce separate variables and if conditions
with spaces around them to make the function call
easier to read.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/storage/storage_util.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 93c24ab6bc..49ecbc5344 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
               unsigned int flags)
 {
     virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+    mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
+    unsigned int flags = 0;
 
     virCheckFlags(0, -1);
 
@@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool,
         return -1;
     }
 
+    if (vol->target.perms->mode != (mode_t)-1)
+        permmode = vol->target.perms->mode;
+
+    if (def->type == VIR_STORAGE_POOL_NETFS)
+        flags |= VIR_DIR_CREATE_AS_UID;
 
     if (virDirCreate(vol->target.path,
-                     (vol->target.perms->mode == (mode_t)-1 ?
-                      VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
-                      vol->target.perms->mode),
+                     permmode,
                      vol->target.perms->uid,
                      vol->target.perms->gid,
-                     (def->type == VIR_STORAGE_POOL_NETFS
-                      ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
+                     flags) < 0) {
         return -1;
     }
 
-- 
2.26.2

Re: [libvirt PATCH 11/14] storage: createFileDir: use less ternary operators

Posted by Ján Tomko 5 years, 4 months ago

On a Wednesday in 2020, Ján Tomko wrote:
>Introduce separate variables and if conditions
>with spaces around them to make the function call
>easier to read.
>
>Signed-off-by: Ján Tomko <jtomko@redhat.com>
>---
> src/storage/storage_util.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>index 93c24ab6bc..49ecbc5344 100644
>--- a/src/storage/storage_util.c
>+++ b/src/storage/storage_util.c
>@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
>               unsigned int flags)

This function already has flags ^

> {
>     virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
>+    mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
>+    unsigned int flags = 0;

Consider
s/flags/createflags/
squashed in from here on

Jano

Re: [libvirt PATCH 11/14] storage: createFileDir: use less ternary operators

Posted by Martin Kletzander 5 years, 4 months ago

On Wed, Sep 23, 2020 at 08:15:00PM +0200, Ján Tomko wrote:
>Introduce separate variables and if conditions
>with spaces around them to make the function call
>easier to read.
>
>Signed-off-by: Ján Tomko <jtomko@redhat.com>
>---
> src/storage/storage_util.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
>index 93c24ab6bc..49ecbc5344 100644
>--- a/src/storage/storage_util.c
>+++ b/src/storage/storage_util.c
>@@ -1997,6 +1997,8 @@ createFileDir(virStoragePoolObjPtr pool,
>               unsigned int flags)
> {
>     virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
>+    mode_t permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;
>+    unsigned int flags = 0;
>
>     virCheckFlags(0, -1);
>
>@@ -2013,15 +2015,17 @@ createFileDir(virStoragePoolObjPtr pool,
>         return -1;
>     }
>
>+    if (vol->target.perms->mode != (mode_t)-1)
>+        permmode = vol->target.perms->mode;
>+

It would be even less repetitive if you did:

     mode_t permmode = vol->target.perms->mode;
     if (permmode == -1)
         permmode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE;

But

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

with your squash, of course.

>+    if (def->type == VIR_STORAGE_POOL_NETFS)
>+        flags |= VIR_DIR_CREATE_AS_UID;
>
>     if (virDirCreate(vol->target.path,
>-                     (vol->target.perms->mode == (mode_t)-1 ?
>-                      VIR_STORAGE_DEFAULT_VOL_PERM_MODE :
>-                      vol->target.perms->mode),
>+                     permmode,
>                      vol->target.perms->uid,
>                      vol->target.perms->gid,
>-                     (def->type == VIR_STORAGE_POOL_NETFS
>-                      ? VIR_DIR_CREATE_AS_UID : 0)) < 0) {
>+                     flags) < 0) {
>         return -1;
>     }
>
>-- 
>2.26.2
>