Opened 11 years ago

Closed 11 years ago

Last modified 11 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 11 years ago.
implementation of `file-creation-mode' (untested)
umask-test.scm (678 bytes) - added by Mario Domenech Goulart 11 years ago.
Simple tests for umask

Download all attachments as: .zip

Change History (25)

comment:1 Changed 11 years ago by Mario Domenech Goulart

Component: unknowncore libraries

comment:2 Changed 11 years ago by felix winkelmann

Milestone: 4.7.0

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

comment:3 Changed 11 years ago by 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.

comment:4 Changed 11 years ago by Christian Kellermann

+1 for the bang
+1 for the posix unit

comment:5 in reply to:  3 Changed 11 years ago by felix winkelmann

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 Changed 11 years ago by Mario Domenech Goulart

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 Changed 11 years ago by felix winkelmann

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)?

comment:8 in reply to:  7 Changed 11 years ago by felix winkelmann

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 Changed 11 years ago by Mario Domenech Goulart

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 Changed 11 years ago by sjamaan

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.

Changed 11 years ago by felix winkelmann

Attachment: file-creation-mode.diff added

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

comment:11 Changed 11 years ago by felix winkelmann

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

comment:12 Changed 11 years ago by felix winkelmann

Should we do a poll on the attached implementation?

comment:14 in reply to:  13 ; Changed 11 years ago by 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.

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

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 Changed 11 years ago by Jim Ursetto

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

comment:18 in reply to:  16 Changed 11 years ago by felix winkelmann

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 Changed 11 years ago by Jim Ursetto

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

comment:20 Changed 11 years ago by felix winkelmann

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

comment:21 Changed 11 years ago by Mario Domenech Goulart

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 Changed 11 years ago by felix winkelmann

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

Changed 11 years ago by Mario Domenech Goulart

Attachment: umask-test.scm added

Simple tests for umask

comment:23 Changed 11 years ago by Mario Domenech Goulart

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.