Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#424 closed change request (fixed)

umask support

Reported by: Mario Domenech Goulart Owned by:
Priority: major Milestone:
Component: core libraries Version: 4.6.x
Keywords: umask Cc:
Estimated difficulty:

Description

Chicken currently doesn't provide bindings to umask(2). It would be nice to have a procedure for that.

Here's the proposed syntax and semantics:

(set-file-creation-mode-mask mask)

Returns the previous value of the file mode mask.

mask can be specified using Unit posix's perm/* constants.

Initial discussion on chicken-hackers proposes adding umask support to the posix unit.

Attachments (2)

file-creation-mode.diff (3.1 KB ) - added by felix winkelmann 15 years ago.
implementation of `file-creation-mode' (untested)
umask-test.scm (678 bytes ) - added by Mario Domenech Goulart 15 years ago.
Simple tests for umask

Download all attachments as: .zip

Change History (25)

comment:1 by Mario Domenech Goulart, 15 years ago

Component: unknowncore libraries

comment:2 by felix winkelmann, 15 years ago

Milestone: 4.7.0

Looks ok. I would add a bang ("!"), though.

comment:3 by sjamaan, 15 years ago

+1 on on the bang.

I'd also like a way to retrieve the current value (file-creation-mode-mask), so you can easily add to it or remove from it, but POSIX sucks and doesn't provide an easy way to do that. It should be possible to set to a dummy value and then immediately reset the umask to fetch the value, though. It would also be a nice candidate for an SRFI-17 setter if we add this.

comment:4 by Christian Kellermann, 15 years ago

+1 for the bang
+1 for the posix unit

in reply to:  3 comment:5 by felix winkelmann, 15 years ago

Replying to sjamaan:

+1 on on the bang.

I'd also like a way to retrieve the current value (file-creation-mode-mask), so you can easily add to it or remove from it, but POSIX sucks and doesn't provide an easy way to do that. It should be possible to set to a dummy value and then immediately reset the umask to fetch the value, though. It would also be a nice candidate for an SRFI-17 setter if we add this.

I also think this is preferable, so an accessor procedure like this would be enough (with a setter).

comment:6 by Mario Domenech Goulart, 15 years ago

Peter suggested taking a look at how scsh implements umask. The API documentation for umask in scsh is http://www.scsh.net/docu/html/man-Z-H-4.html#node_sec_3.5

As far as I understand, scsh's umask is thread-safe (i.e., threads can set different masks).

The primitive for getting the current mask is implemented like Peter suggests:

(define (process-umask)
  (let ((m (set-process-umask 0)))
    (set-process-umask m)
    m))

comment:7 by felix winkelmann, 15 years ago

I think we can do the poll now, since there isn't much to be discussed anymore.
So, mario, if it's ok with you, I ...

Poll(UMASK!!!; Yes; No)

in reply to:  7 comment:8 by felix winkelmann, 15 years ago

Replying to felix:

I think we can do the poll now, since there isn't much to be discussed anymore.
So, mario, if it's ok with you, I ...

Poll(UMASK!!!; Yes; No)

BTW, we should announce this on chicken-hackers.

comment:9 by Mario Domenech Goulart, 15 years ago

Hi Felix. Thanks for bringing this up again and sorry for being quiet lately.

My concerns are regarding to the behavior of umask in a multi-threaded environment and the API.

Should it be possible to allow multiple threads to set different file creation modes? As far as I understand, Scsh allows that (I may be wrong, though).

How should the umask support be implemented? As Peter suggests (i.e., having file-creation-mode-mask and a setter for it)? Or just set-file-creation-mode-mask! which is just a direct binding to the umask syscall?

Maybe the poll should contain more options, like:

  • I want umask support via file-creation-mode and a setter for it. Multiple threads share the same mask.
  • I want umask support via file-creation-mode and a setter for it. Each thread can set its own mask.
  • I want umask support via set-file-creation-mode-mask! (it changes the mask and returns the old mask -- a direct binding to the umask syscall). Each thread can set its own mask.
  • I want umask support via set-file-creation-mode-mask! (it changes the mask and returns the old mask -- a direct binding to the umask syscall). Multiple threads share the same mask.
  • I don't want umask support

Or maybe we can discuss those options and reduce them to a smaller set before setting a poll (suggestion). Maybe some of my concerns are just silly and can be safely ignored.

comment:10 by sjamaan, 15 years ago

I'd welcome discussion on chicken-hackers about this. The per-thread option is definitely hairy enough to warrant additional discussion because that would require a call to umask() on each context switch. I don't know how expensive such a call is, but I can imagine it isn't the cheapest thing to do.

by felix winkelmann, 15 years ago

Attachment: file-creation-mode.diff added

implementation of `file-creation-mode' (untested)

comment:11 by felix winkelmann, 15 years ago

I added an implementation for the accessor + setter case. Note that this is currently untested.

comment:12 by felix winkelmann, 15 years ago

Should we do a poll on the attached implementation?

in reply to:  13 ; comment:14 by sjamaan, 15 years ago

Replying to mario:

Poll(Do you think we should adopt the patch provided by Felix to implement umask support?; Yes; No)

Small nitpick: The manual section should probably look like this:

<procedure>(file-creation-mode)</procedure>
<setter>(set! (file-creation-mode) MODE)</setter>

The <procedure>(file-creation-mode MODE)</procedure> implies you can change the mode by calling the file-creation-mode getter procedure with a MODE argument.

in reply to:  14 comment:15 by Alaric Snell-Pym, 15 years ago

Replying to sjamaan:

Replying to mario:

Poll(Do you think we should adopt the patch provided by Felix to implement umask support?; Yes; No)

Small nitpick: The manual section should probably look like this:

<procedure>(file-creation-mode)</procedure>
<setter>(set! (file-creation-mode) MODE)</setter>

The <procedure>(file-creation-mode MODE)</procedure> implies you can change the mode by calling the file-creation-mode getter procedure with a MODE argument.

Agreed!

comment:16 by Jim Ursetto, 15 years ago

I'm going to vote yes, but did anyone actually test this? (it says untested and the poll is for this patch specifically)

in reply to:  16 comment:18 by felix winkelmann, 15 years ago

Replying to zbigniew:

I'm going to vote yes, but did anyone actually test this? (it says untested and the poll is for this patch specifically)

The poll is for this way of implementing it (at least that's I understand it). If we start polling over whether some code works or not, we're not gonna get far...

So, if it doesn't work, we'll fix it - we always do it like that! ;-)

comment:19 by Jim Ursetto, 15 years ago

Oh ok. I was confused by the "implementation of the patch" wording.

comment:20 by felix winkelmann, 15 years ago

Can we assume this is accepted? (there are no objections, it seems, even if Kon and Ivan didn't vote)

comment:21 by Mario Domenech Goulart, 15 years ago

Resolution: fixed
Status: newclosed

I think so (from /cr-proposal: "The voting period should not exceed one week."). We are late, actually. :-) I'm closing this ticket.

comment:22 by felix winkelmann, 15 years ago

I applied the patch. Mario, could you verify that it works as intended? I never use this operation.

by Mario Domenech Goulart, 15 years ago

Attachment: umask-test.scm added

Simple tests for umask

comment:23 by Mario Domenech Goulart, 15 years ago

Hi Felix

Thank you. I had tested the patch when you first made it available. I've tested it again using experimental (8a65dc646b3848ce0a6100c017bfe900d54231e2). It seems to work as expected. I've made a very simple test file for testing umask (http://bugs.call-cc.org/attachment/ticket/424/umask-test.scm). Sorry for the long time to reply.

Note: See TracTickets for help on using tickets.