On 02/04/2013 10:48 AM, Jakub Filak wrote:
> Thanks. Pushed.
> 
> commit cc2e4db36f25c32a118e2c4b06c719d636c3196f
> Author: Jakub Filak <[email protected]>
> Date:   Mon Feb 4 09:13:43 2013 +0100
> 
>     dd: always sanitize mode of saved file
>     
>     - we use O_CREAT as the argument for open() and man says:
>         'The permissions of the created file are (mode & ~umask)'

This is true only if we did create file. We are not sure we created it
in this case - it may exist already. Let's not mislead people by
the above text in the log.

>     - we want to set the required mode without mangling it by umask, thus we 
> have to
>       change file's mode by chmod

Good :)

>     - related to trac#927
>     
>     Signed-off-by: Jakub Filak <[email protected]>
> 
> diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
> index 37606dd..213236c 100644
> --- a/src/lib/dump_dir.c
> +++ b/src/lib/dump_dir.c
> @@ -893,6 +893,15 @@ static bool save_binary_file(const char *path, const 
> char* 
> data, unsigned size,
>          }
>      }
>  
> +    /* O_CREATE in the open() call above causes that the permissions of the
> +     * created file are (mode & ~umask) */
> +    if (fchmod(fd, mode) == -1)
> +    {
> +        perror_msg("Can't change mode of '%s'", path);
> +        close(fd);
> +        return false;
> +    }

Looks inconsistent with what we do if *fchown* fails:

        if (fchown(fd, uid, gid) == -1)
        {
            perror_msg("Can't change '%s' ownership to %lu:%lu", path, 
(long)uid, (long)gid);
        }

I think it's better to do the same thing (whatever that "thing" is)
in both cases.


-- 
vda

Reply via email to