Re: [libvirt] [PATCH v3 05/14] util: Implement virFileChownFiles()

[John Ferlan]

On 05/04/2018 04:21 PM, Stefan Berger wrote:
> Implement virFileChownFiles() which changes file ownership of all
> files in a given directory.
> 
> Signed-off-by: Stefan Berger 
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virfile.c   | 49 
> 
>  src/util/virfile.h   |  3 +++
>  3 files changed, 53 insertions(+)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index f2a4921..33fe75b 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1761,6 +1761,7 @@ virFileActivateDirOverride;
>  virFileBindMountDevice;
>  virFileBuildPath;
>  virFileCanonicalizePath;
> +virFileChownFiles;
>  virFileClose;
>  virFileComparePaths;
>  virFileCopyACLs;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 526b9ad..b6aaf2c 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -38,6 +38,7 @@
>  #include 
>  #include 
>  #include 
> +#include 

I take it we don't need ftw.h any more...

>  #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
>  # include 
>  #endif
> @@ -2949,6 +2950,54 @@ void virDirClose(DIR **dirp)
>  *dirp = NULL;
>  }
>  

Two empty lines

> +/*
> + * virFileChownFiles:
> + * @name: name of the directory
> + * @uid: uid
> + * @gid: gid
> + *
> + * Change ownership of all regular files in a directory.
> + *
> + * Returns -1 on error, with error already reported, 0 on success.
> + */
> +int virFileChownFiles(const char *name, uid_t uid, gid_t gid)

One arg per line.

> +{
> +struct dirent *ent;
> +int ret;

s/ret;/ret = -1;/

int direrr;

> +DIR *dir;
> +char *path;

path = NULL;

> +
> +if (virDirOpen(, name) < 0)
> +return -1;
> +
> +while ((ret = virDirRead(dir, , name)) > 0) {

s/ret/direrr

> +if (ent->d_type != DT_REG)
> +continue;
> +
> +if (virAsprintf(, "%s/%s", name, ent->d_name) < 0) {
> +ret = -1;
> +break;
> +}

Replace {...} w/

goto cleanup;

> +if (chown(path, uid, gid) < 0) {
> +ret = -1;

Unnecessary.

> +virReportSystemError(errno,
> + _("cannot chown '%s' to (%u, %u)"),
> + ent->d_name, (unsigned int) uid,
> + (unsigned int) gid);
   goto cleanup;

> +}
> +VIR_FREE(path);
> +if (ret < 0)
> +break;

Unnecessary

> +}
> +

if (direrr < 0)
goto cleanup;

ret = 0;

 cleanup:
VIR_FREE(path);

> +virDirClose();

return ret;

Skip the rest.

> +
> +if (ret < 0)
> +return -1;
> +
> +return 0;
> +}
> +

Two empty lines

With the adjustments,

Reviewed-by: John Ferlan 

John

>  static int
>  virFileMakePathHelper(char *path, mode_t mode)
>  {
> diff --git a/src/util/virfile.h b/src/util/virfile.h
> index 13d3cf6..f0d99a0 100644
> --- a/src/util/virfile.h
> +++ b/src/util/virfile.h
> @@ -239,6 +239,9 @@ int virFileOpenAs(const char *path, int openflags, mode_t 
> mode,
>  ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>  int virFileRemove(const char *path, uid_t uid, gid_t gid);
>  
> +int virFileChownFiles(const char *name, uid_t uid, gid_t gid)
> +ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> +
>  enum {
>  VIR_DIR_CREATE_NONE= 0,
>  VIR_DIR_CREATE_AS_UID  = (1 << 0),
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 05/14] util: Implement virFileChownFiles()

[Stefan Berger]

Implement virFileChownFiles() which changes file ownership of all
files in a given directory.

Signed-off-by: Stefan Berger 
---
 src/libvirt_private.syms |  1 +
 src/util/virfile.c   | 49 
 src/util/virfile.h   |  3 +++
 3 files changed, 53 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f2a4921..33fe75b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1761,6 +1761,7 @@ virFileActivateDirOverride;
 virFileBindMountDevice;
 virFileBuildPath;
 virFileCanonicalizePath;
+virFileChownFiles;
 virFileClose;
 virFileComparePaths;
 virFileCopyACLs;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 526b9ad..b6aaf2c 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
 # include 
 #endif
@@ -2949,6 +2950,54 @@ void virDirClose(DIR **dirp)
 *dirp = NULL;
 }
 
+/*
+ * virFileChownFiles:
+ * @name: name of the directory
+ * @uid: uid
+ * @gid: gid
+ *
+ * Change ownership of all regular files in a directory.
+ *
+ * Returns -1 on error, with error already reported, 0 on success.
+ */
+int virFileChownFiles(const char *name, uid_t uid, gid_t gid)
+{
+struct dirent *ent;
+int ret;
+DIR *dir;
+char *path;
+
+if (virDirOpen(, name) < 0)
+return -1;
+
+while ((ret = virDirRead(dir, , name)) > 0) {
+if (ent->d_type != DT_REG)
+continue;
+
+if (virAsprintf(, "%s/%s", name, ent->d_name) < 0) {
+ret = -1;
+break;
+}
+if (chown(path, uid, gid) < 0) {
+ret = -1;
+virReportSystemError(errno,
+ _("cannot chown '%s' to (%u, %u)"),
+ ent->d_name, (unsigned int) uid,
+ (unsigned int) gid);
+}
+VIR_FREE(path);
+if (ret < 0)
+break;
+}
+
+virDirClose();
+
+if (ret < 0)
+return -1;
+
+return 0;
+}
+
 static int
 virFileMakePathHelper(char *path, mode_t mode)
 {
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 13d3cf6..f0d99a0 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -239,6 +239,9 @@ int virFileOpenAs(const char *path, int openflags, mode_t 
mode,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
 int virFileRemove(const char *path, uid_t uid, gid_t gid);
 
+int virFileChownFiles(const char *name, uid_t uid, gid_t gid)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+
 enum {
 VIR_DIR_CREATE_NONE= 0,
 VIR_DIR_CREATE_AS_UID  = (1 << 0),
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list