Tom Lane wrote:
> Itagaki Takahiro <itagaki.takah...@gmail.com> writes:
>> ...
>> We should also check the value not to be something like 0699.
>> How about checking it with (file_mode & ~0666) != 0 ?
>> ...
>> I want show_log_file_mode to print the setting value in octal format.
> 
> It seems like a whole lot of lily-gilding is going on here.  Just make
> it work like unix_socket_permissions already does.  That's been there
> for years and nobody has complained about it.
> 

Thanks, somehow I missed that we can already specify octal integers
as GUC-s. I now converted the log_file_mode to integer and dropped
the assign_log_file_mode function.

regards,
Martin
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 2789,2794 **** local0.*    /var/log/postgresql
--- 2789,2817 ----
        </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 parameter value is expected to be a numeric mode
+         specified in the format accepted by the
+         <function>chmod</function> and <function>umask</function>
+         system calls.  (To use the customary octal format the number
+         must start with a <literal>0</literal> (zero).)
+        </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/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
***************
*** 174,179 **** static bool assign_autovacuum_max_workers(int newval, bool doit, GucSource sourc
--- 174,180 ----
  static bool assign_effective_io_concurrency(int newval, bool doit, GucSource source);
  static const char *assign_pgstat_temp_directory(const char *newval, bool doit, GucSource source);
  static const char *assign_application_name(const char *newval, bool doit, GucSource source);
+ static const char *show_log_file_mode(void);
  
  static char *config_enum_get_options(struct config_enum * record,
  						const char *prefix, const char *suffix,
***************
*** 2068,2073 **** static struct config_int ConfigureNamesInt[] =
--- 2069,2083 ----
  		1024, 100, 102400, NULL, NULL
  	},
  
+ 	{
+ 		{"log_file_mode", PGC_SIGHUP, LOGGING_WHERE,
+ 			gettext_noop("Sets the file permissions for log files."),
+ 			NULL
+ 		},
+ 		&Log_file_mode,
+ 		0600, 0000, 0777, NULL, show_log_file_mode
+ 	},
+ 
  	/* End-of-list marker */
  	{
  		{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL
***************
*** 8072,8075 **** assign_application_name(const char *newval, bool doit, GucSource source)
--- 8082,8094 ----
  		return newval;
  }
  
+ static const char *
+ show_log_file_mode(void)
+ {
+ 	static char buf[5];
+ 
+ 	snprintf(buf, sizeof(buf), "%04o", Log_file_mode);
+ 	return buf;
+ }
+ 
  #include "guc-file.c"
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 265,270 ****
--- 265,272 ----
  					# 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,
+ 					# begin with 0 to use octal notation
  #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/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

Reply via email to