Peter Eisentraut <peter.eisentr...@2ndquadrant.com> writes: > I wonder whether we even need that much flexibility. We already set a > global umask, so we could just open all files with 0666 and let the > umask sort it out. Then we don't need all the *Perm() variants.
-1. What that would mean is that if somebody had a need for a nonstandard file mask, the path of least resistance would be to bypass fd.c and open the file directly. We do *not* want to encourage that. I think David's design with macros inserting the default permission choices looks fine (but I haven't read the patch completely). > I also wonder whether the umask save-and-restore code in copy.c and > be-fsstubs.c is sound. If the AllocateFile() call in between errors > out, then there is nothing that restores the original umask. This might > need a TRY/CATCH block, or maybe just a chmod() afterwards. Yeah, this seems like a problem. > This kind of code: > - if (stat_buf.st_mode & (S_IRWXG | S_IRWXO)) > + if (stat_buf.st_mode & PG_MODE_MASK_DEFAULT) > ereport(FATAL, > (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), > errmsg("data directory \"%s\" has group or world access", > DataDir), > - errdetail("Permissions should be u=rwx (0700)."))); > + errdetail("Permissions should be (%04o).", > PG_DIR_MODE_DEFAULT))); I think this hunk should be left entirely alone. The goal of this patch should not be "eliminate every reference to S_IRWXO", anyway. Trying to replace this one seems like it can have no good effect: it would mean that if anyone ever changes PG_MODE_MASK_DEFAULT, they've silently introduced either a security hole or a overly restrictive check. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers