Itagaki Takahiro wrote:
> I think the patch is almost ready for committer except the following
> three issues:
>
> 2010/7/13 Fujii Masao <masao.fu...@gmail.com>:
>>> + if (!*value || *endptr || file_mode < 0 || file_mode > 0777)
>> The sticky bit cannot be set via log_file_mode. Is this intentional?
Yes -- I don't think there is a use case for sticky or setuid bits on log
files, even allowing execute is questionable.
> We should also check the value not to be something like 0699.
> How about checking it with (file_mode & ~0666) != 0 ?
Aha, that would ensure that the execute bit is not specified. Works for me.
The 0699 and other invalid octal values are caught by strtol()
>>> +#ifndef WIN32
>>> + chmod(filename, Log_file_mode);
>>> +#endif
>> Don't we need to check the return value of chmod()?
>
> I prefer umask() rather than chmod() here.
>
Converted to umask()
>>> +const char *show_log_file_mode(void);
>> You forgot to write the show_log_file_mode()? I was not able to find that
>> in the patch.
>
> I want show_log_file_mode to print the setting value in octal format.
>
I've now (re)added the show_log_file_mode(). It used to be there, but then
at some point I decided to display the value "as-is".
While going through it, I moved the _setmode() call for win32 to logfile_open().
regards,
Martin
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 2789,2794 **** local0.* /var/log/postgresql
--- 2789,2815 ----
</listitem>
</varlistentry>
+ <varlistentry id="guc-log-file-mode" xreflabel="log_file_mode">
+ <term><varname>log_file_mode</varname> (<type>integer</type>)</term>
+ <indexterm>
+ <primary><varname>log_file_mode</> configuration parameter</primary>
+ </indexterm>
+ <listitem>
+ <para>
+ On Unix systems this parameter sets the permissions for log files
+ when <varname>logging_collector</varname> is enabled. On Microsoft
+ Windows the file mode will be ignored. The value is an octal number
+ consisting of 3 digits signifying the permissions for the user, group
+ and others. Specifying execute permissions for log files results in
+ an error.
+ </para>
+ <para>
+ This parameter can only be set in the <filename>postgresql.conf</>
+ file or on the server command line.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-log-rotation-age" xreflabel="log_rotation_age">
<term><varname>log_rotation_age</varname> (<type>integer</type>)</term>
<indexterm>
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 914,916 **** show_role(void)
--- 914,956 ----
return endptr + 1;
}
+
+
+ /*
+ * LOG_FILE_MODE
+ */
+
+ extern int Log_file_mode; /* in guc.c */
+
+ /*
+ * assign_log_file_mode: GUC assign_hook for log_file_mode
+ */
+ const char *
+ assign_log_file_mode(const char *value, bool doit, GucSource source)
+ {
+ char *endptr;
+ long file_mode = strtol(value, &endptr, 8);
+
+ if (!*value || *endptr || file_mode < 0 || (file_mode & ~0666) != 0)
+ {
+ ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"log_file_mode\""),
+ errhint("value must be a 3 digit octal number, with the execute bit not set")));
+ return NULL;
+ }
+
+ if (doit)
+ Log_file_mode = (int) file_mode;
+
+ return value;
+ }
+
+ const char *
+ show_log_file_mode(void)
+ {
+ static char buf[5];
+
+ snprintf(buf, sizeof(buf), "%04o", Log_file_mode);
+ return buf;
+ }
*** a/src/backend/postmaster/syslogger.c
--- b/src/backend/postmaster/syslogger.c
***************
*** 73,78 **** int Log_RotationSize = 10 * 1024;
--- 73,79 ----
char *Log_directory = NULL;
char *Log_filename = NULL;
bool Log_truncate_on_rotation = false;
+ int Log_file_mode = 0600;
/*
* Globally visible state (used by elog.c)
***************
*** 135,140 **** static void syslogger_parseArgs(int argc, char *argv[]);
--- 136,142 ----
static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
static void open_csvlogfile(void);
+ static FILE *logfile_open(const char *filename, const char *mode, bool am_rotating);
#ifdef WIN32
static unsigned int __stdcall pipeThread(void *arg);
***************
*** 516,530 **** SysLogger_Start(void)
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = fopen(filename, "a");
!
! if (!syslogFile)
! ereport(FATAL,
! (errcode_for_file_access(),
! (errmsg("could not create log file \"%s\": %m",
! filename))));
!
! setvbuf(syslogFile, NULL, LBF_MODE, 0);
pfree(filename);
--- 518,524 ----
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = logfile_open(filename, "a", false);
pfree(filename);
***************
*** 1004,1027 **** open_csvlogfile(void)
filename = logfile_getname(time(NULL), ".csv");
! fh = fopen(filename, "a");
! if (!fh)
! ereport(FATAL,
! (errcode_for_file_access(),
! (errmsg("could not create log file \"%s\": %m",
! filename))));
! setvbuf(fh, NULL, LBF_MODE, 0);
! #ifdef WIN32
! _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
! #endif
! csvlogFile = fh;
! pfree(filename);
}
/*
--- 998,1050 ----
filename = logfile_getname(time(NULL), ".csv");
! fh = logfile_open(filename, "a", false);
! csvlogFile = fh;
! pfree(filename);
! }
! /*
! * Open the logfile with proper permissions and buffering options.
! *
! * We ignore any errors if the logfile is opened as part of log rotation. In
! * other cases the errors are treated as fatal.
! */
! static FILE *
! logfile_open(const char *filename, const char *mode, bool am_rotating)
! {
! mode_t oumask;
! FILE *fh;
! oumask = umask((mode_t) (~Log_file_mode & 0777));
! fh = fopen(filename, mode);
! umask(oumask);
!
! if (fh)
! {
! setvbuf(fh, NULL, LBF_MODE, 0);
+ #ifdef WIN32
+ _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
+ #endif
+ }
+ else
+ {
+ if (am_rotating)
+ ereport(LOG,
+ (errcode_for_file_access(),
+ errmsg("could not create new log file \"%s\": %m",
+ filename)));
+ else
+ ereport(FATAL,
+ (errcode_for_file_access(),
+ errmsg("could not create log file \"%s\": %m",
+ filename)));
+ }
+
+ return fh;
}
/*
***************
*** 1070,1088 **** logfile_rotate(bool time_based_rotation, int size_rotation_for)
if (Log_truncate_on_rotation && time_based_rotation &&
last_file_name != NULL &&
strcmp(filename, last_file_name) != 0)
! fh = fopen(filename, "w");
else
! fh = fopen(filename, "a");
if (!fh)
{
int saveerrno = errno;
- ereport(LOG,
- (errcode_for_file_access(),
- errmsg("could not open new log file \"%s\": %m",
- filename)));
-
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
--- 1093,1106 ----
if (Log_truncate_on_rotation && time_based_rotation &&
last_file_name != NULL &&
strcmp(filename, last_file_name) != 0)
! fh = logfile_open(filename, "w", true);
else
! fh = logfile_open(filename, "a", true);
if (!fh)
{
int saveerrno = errno;
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
***************
*** 1104,1115 **** logfile_rotate(bool time_based_rotation, int size_rotation_for)
return;
}
- setvbuf(fh, NULL, LBF_MODE, 0);
-
- #ifdef WIN32
- _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
- #endif
-
fclose(syslogFile);
syslogFile = fh;
--- 1122,1127 ----
***************
*** 1128,1146 **** logfile_rotate(bool time_based_rotation, int size_rotation_for)
if (Log_truncate_on_rotation && time_based_rotation &&
last_csv_file_name != NULL &&
strcmp(csvfilename, last_csv_file_name) != 0)
! fh = fopen(csvfilename, "w");
else
! fh = fopen(csvfilename, "a");
if (!fh)
{
int saveerrno = errno;
- ereport(LOG,
- (errcode_for_file_access(),
- errmsg("could not open new log file \"%s\": %m",
- csvfilename)));
-
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
--- 1140,1153 ----
if (Log_truncate_on_rotation && time_based_rotation &&
last_csv_file_name != NULL &&
strcmp(csvfilename, last_csv_file_name) != 0)
! fh = logfile_open(csvfilename, "w", true);
else
! fh = logfile_open(csvfilename, "a", true);
if (!fh)
{
int saveerrno = errno;
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
***************
*** 1162,1173 **** logfile_rotate(bool time_based_rotation, int size_rotation_for)
return;
}
- setvbuf(fh, NULL, LBF_MODE, 0);
-
- #ifdef WIN32
- _setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
- #endif
-
fclose(csvlogFile);
csvlogFile = fh;
--- 1169,1174 ----
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 412,417 **** static char *server_version_string;
--- 412,418 ----
static int server_version_num;
static char *timezone_string;
static char *log_timezone_string;
+ static char *log_file_mode_string;
static char *timezone_abbreviations_string;
static char *XactIsoLevel_string;
static char *data_directory;
***************
*** 2243,2248 **** static struct config_string ConfigureNamesString[] =
--- 2244,2258 ----
},
{
+ {"log_file_mode", PGC_SIGHUP, LOGGING_WHERE,
+ gettext_noop("Sets the file permissions for log files."),
+ NULL
+ },
+ &log_file_mode_string,
+ "0600", assign_log_file_mode, show_log_file_mode
+ },
+
+ {
{"DateStyle", PGC_USERSET, CLIENT_CONN_LOCALE,
gettext_noop("Sets the display format for date and time values."),
gettext_noop("Also controls interpretation of ambiguous "
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 265,270 ****
--- 265,271 ----
# can be absolute or relative to PGDATA
#log_filename = 'postgresql-%Y-%m-%d_%H%M%S.log' # log file name pattern,
# can include strftime() escapes
+ #log_file_mode = 0600 # creation mode for log files, octal
#log_truncate_on_rotation = off # If on, an existing log file of the
# same name as the new log file will be
# truncated rather than appended to.
*** a/src/include/commands/variable.h
--- b/src/include/commands/variable.h
***************
*** 35,39 **** extern const char *show_role(void);
--- 35,42 ----
extern const char *assign_session_authorization(const char *value,
bool doit, GucSource source);
extern const char *show_session_authorization(void);
+ const char *assign_log_file_mode(const char *value,
+ bool doit, GucSource source);
+ const char *show_log_file_mode(void);
#endif /* VARIABLE_H */
*** a/src/include/postmaster/syslogger.h
--- b/src/include/postmaster/syslogger.h
***************
*** 68,73 **** extern int Log_RotationSize;
--- 68,74 ----
extern PGDLLIMPORT char *Log_directory;
extern PGDLLIMPORT char *Log_filename;
extern bool Log_truncate_on_rotation;
+ extern int Log_file_mode;
extern bool am_syslogger;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
