On 10/12/2015 06:25 AM, Michal Privoznik wrote:
> It's better if we stat() file that we are about to chown() at
> first and check if there's something we need to change. Not that
> it would make much difference, but for the upcoming patches we
> need to be doing stat() anyway.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> src/security/security_dac.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 0dfe570..9b079e0 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -271,17 +271,18 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr priv,
> path = src->path;
> }
>
> + if (stat(path, &sb) < 0) {
> + virReportSystemError(errno, _("unable to stat: %s"), path);
> + return -1;
> + }
> +
> + if (sb.st_uid == uid && sb.st_gid == gid) {
> + /* nothing to chown */
> + return 0;
> + }
> +
> rc = chown(path, uid, gid);
> chown_errno = errno;
> -
> - if (rc < 0 &&
> - stat(path, &sb) >= 0) {
> - if (sb.st_uid == uid &&
> - sb.st_gid == gid) {
> - /* It's alright, there's nothing to change anyway. */
> - return 0;
> - }
> - }
I don't believe chown_errno is necessary anymore either since a failure
in stat() won't overwrite errno.
Your call to keep it or not since it's also part of the "if" portion
(although that path doesn't overwrite either).
ACK -
John
> }
>
> if (rc < 0) {
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list