[PATCH] fallocate: create mode 0666, that's what umask is for

[PATCH] fallocate: create mode 0666, that's what umask is for

[Peter Cordes]

User's umask will typically mask the mode down to 0664 or 0644.
---
 sys-utils/fallocate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys-utils/fallocate.c b/sys-utils/fallocate.c
index 0e06524b8c5837a63230dc047233c657c50c1d7c..9af3bb8ce1492defda57cc17764197790bb34c8e 100644
--- a/sys-utils/fallocate.c
+++ b/sys-utils/fallocate.c
@@ -365,7 +365,7 @@ int main(int argc, char **argv)
 
 	/* O_CREAT makes sense only for the default fallocate(2) behavior
 	 * when mode is no specified and new space is allocated */
-	fd = open(filename, O_RDWR | (!dig && !mode ? O_CREAT : 0), 0644);
+	fd = open(filename, O_RDWR | (!dig && !mode ? O_CREAT : 0), 0666);
 	if (fd < 0)
 		err(EXIT_FAILURE, _("cannot open %s"), filename);
 
-- 
2.2.1

Re: [PATCH] fallocate: create mode 0666, that's what umask is for

[Karel Zak]

On Tue, Dec 30, 2014 at 01:02:17AM -0400, Peter Cordes wrote:
> diff --git a/sys-utils/fallocate.c b/sys-utils/fallocate.c
> index 0e06524b8c5837a63230dc047233c657c50c1d7c..9af3bb8ce1492defda57cc17764197790bb34c8e 100644
> --- a/sys-utils/fallocate.c
> +++ b/sys-utils/fallocate.c
> @@ -365,7 +365,7 @@ int main(int argc, char **argv)
>  
>  	/* O_CREAT makes sense only for the default fallocate(2) behavior
>  	 * when mode is no specified and new space is allocated */
> -	fd = open(filename, O_RDWR | (!dig && !mode ? O_CREAT : 0), 0644);
> +	fd = open(filename, O_RDWR | (!dig && !mode ? O_CREAT : 0), 0666);

 Applied, but I have replaced the number with 

    S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH

 macros to keep the code more readable.

 Thanks
     Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] fallocate: create mode 0666, that's what umask is for

[Peter Cordes]

On Wed, Jan 07, 2015 at 10:03:54AM +0100, Karel Zak wrote:
> On Tue, Dec 30, 2014 at 01:02:17AM -0400, Peter Cordes wrote:
> > diff --git a/sys-utils/fallocate.c b/sys-utils/fallocate.c
> > index 0e06524b8c5837a63230dc047233c657c50c1d7c..9af3bb8ce1492defda57cc17764197790bb34c8e 100644
> > --- a/sys-utils/fallocate.c
> > +++ b/sys-utils/fallocate.c
> > @@ -365,7 +365,7 @@ int main(int argc, char **argv)
> >  
> >  	/* O_CREAT makes sense only for the default fallocate(2) behavior
> >  	 * when mode is no specified and new space is allocated */
> > -	fd = open(filename, O_RDWR | (!dig && !mode ? O_CREAT : 0), 0644);
> > +	fd = open(filename, O_RDWR | (!dig && !mode ? O_CREAT : 0), 0666);
> 
>  Applied, but I have replaced the number with 
> 
>     S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH
> 
>  macros to keep the code more readable.

 0666 is more readable, to me.  With the macros, I have to stop and
look to see what each one is, and figure out if any bits are left out.
If you were testing one bit in a given permission set, using a macro
would probably be more readable, but 0666 says "everything but execute"
in a lot less time than it takes to mentally OR 6 macros together.

 Maybe I'm weird for normally using numerical args to chmod, rather
than chmod +x, and most people don't chmod 755 often?

 Obviously it's your call in the end, as maintainer, and either way
doesn't make a big difference, since it compiles identically.

 Thanks for taking a look at my patches.  I haven't had any new ideas
for my fallocate --dig-holes patch.  My local copy does what I want it
to, and I haven't thought of anything else I really want.  All it
needs is probably just cleaning up what's printed at various -v
levels, and maybe a hole-size option.

-- 
#define X(x,y) x##y
Peter Cordes ;  e-mail: X(peter@cor , des.ca)

"The gods confound the man who first found out how to distinguish the hours!
 Confound him, too, who in this place set up a sundial, to cut and hack
 my day so wretchedly into small pieces!" -- Plautus, 200 BC

Re: [PATCH] fallocate: create mode 0666, that's what umask is for

[Karel Zak]

On Wed, Jan 07, 2015 at 06:46:26PM -0400, Peter Cordes wrote:
>  Thanks for taking a look at my patches.  I haven't had any new ideas
> for my fallocate --dig-holes patch.  My local copy does what I want it

 I didn't look at your second patch yet. I'd like to finalize stuff for 
 v2.26-rc1 and it seems that with some invasive and large changes we
 can wait for v2.27.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH] fallocate: create mode 0666, that's what umask is for

[Peter Cordes]

On Thu, Jan 08, 2015 at 10:13:02AM +0100, Karel Zak wrote:
> On Wed, Jan 07, 2015 at 06:46:26PM -0400, Peter Cordes wrote:
> >  Thanks for taking a look at my patches.  I haven't had any new ideas
> > for my fallocate --dig-holes patch.  My local copy does what I want it
> 
>  I didn't look at your second patch yet. I'd like to finalize stuff for 
>  v2.26-rc1 and it seems that with some invasive and large changes we
>  can wait for v2.27.

 Sure, no rush.  Agree it's not something that's ready to go in for a
release you're already finalizing.

  I only just subscribed to the mailing list to post my patches.
Thanks for the heads up on release-schedule issues, so I can stop
worrying that you missed it or something.  :)

  If anyone has any thoughts about the design, esp. from the point of
view of a different use-case for --dig-holes (like maybe for VM
images?), that would be useful.  I could maybe revise my patch and
have something closer to ready for inclusion into 2.27.

-- 
#define X(x,y) x##y
Peter Cordes ;  e-mail: X(peter@cor , des.ca)

"The gods confound the man who first found out how to distinguish the hours!
 Confound him, too, who in this place set up a sundial, to cut and hack
 my day so wretchedly into small pieces!" -- Plautus, 200 BC