Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#424 closed change request (fixed)

umask support

Reported by: mario 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 9 years ago.
implementation of `file-creation-mode' (untested)
umask-test.scm (678 bytes) - added by mario 9 years ago.
Simple tests for umask

Download all attachments as: .zip

Change History (25)

comment:1 Changed 9 years ago by mario

  • Component changed from unknown to core libraries

comment:2 Changed 9 years ago by felix

  • Milestone 4.7.0 deleted

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

comment:3 follow-up: Changed 9 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 9 years ago by ckeen

+1 for the bang
+1 for the posix unit

comment:5 in reply to: ↑ 3 Changed 9 years ago by felix

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 9 years ago by mario

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 follow-up: Changed 9 years ago by 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)?

comment:8 in reply to: ↑ 7 Changed 9 years ago by felix

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 9 years ago by mario

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 9 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 9 years ago by felix

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

comment:11 Changed 9 years ago by felix

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

comment:12 Changed 9 years ago by felix

Should we do a poll on the attached implementation?

comment:14 in reply to: ↑ 13 ; follow-up: Changed 9 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 9 years ago by alaric

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 follow-up: Changed 9 years ago by zbigniew

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 9 years ago by felix

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 9 years ago by zbigniew

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

comment:20 Changed 9 years ago by felix

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

comment:21 Changed 9 years ago by mario

  • Resolution set to fixed
  • Status changed from new to closed

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 9 years ago by felix

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

Changed 9 years ago by mario

Simple tests for umask

comment:23 Changed 9 years ago by mario

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.