Viktor Dukhovni:
> On Wed, Feb 11, 2015 at 06:17:13PM -0800, Corey Ashford wrote:
>
> > From our reading of the code, tag can never be 0 there, so that makes the
> > "then" part of the if statement dead code.
> >
> > After that, there's another if statement (line 254) that will always
> > evaluate as true:
> >
> > if (tag != 0) {
> > ...
> >
> >
> > In summary, I believe that removing the "tag = 0;" line was not the right
> > fix for the seg fault bug, but it's not clear to me what the right fix is.
>
> I would:
The correct fix is a little trickier than that.
Many Postfix command-line utilities have code like this:
/* Part 1 - preliminary logging setup. */
if ((slash = strrchr(argv[0], '/')) != 0 && slash[1])
argv[0] = slash + 1;
msg_vstream_init(argv[0], VSTREAM_ERR);
msg_syslog_init(mail_task(argv[0]), LOG_PID, LOG_FACILITY);
...
/* Part 2: Parse JCL */
...
/* Part 3 - read main.cf and finalize logging setup. */
mail_conf_read();
if (strcmp(var_syslog_name, DEF_SYSLOG_NAME) != 0)
/* missing msg_vstream_init() call here! */
msg_syslog_init(mail_task(argv[0]), LOG_PID, LOG_FACILITY);
First, there is some clumsines in part 1, because mail_task()
also contains code to skip over slashes in the command name.
Second, the code in part 3 has had a bogus precondition for the
past six years: it compares var_syslog_name against DEF_SYSLOG_NAME,
the latter of which now contains macro expressions. The two strings
cannot ever match, and msg_syslog_init() is always called.
Correcting the precondition for the msg_syslog_init() call requires
information that Postfix applications should not be concerned with
(they would have to know how the result from mail_task() depends
on the MAIL_LOGTAG environment variable). The correct fix then is
to remove the bogus precondition altogether, and save a few cycles.
With this, the patch for postlog is below. The bogus precondition
and unnecessary slash parsing can also be cleaned up in the other
Postfix command-line utilities.
Wietse
Context diff, for readability.
*** /var/tmp/postfix-3.1-20150208/src/postlog/postlog.c 2012-02-14
09:02:23.000000000 -0500
--- postlog.c 2015-02-12 19:35:29.000000000 -0500
***************
*** 171,175 ****
{
struct stat st;
- char *slash;
int fd;
int ch;
--- 171,174 ----
***************
*** 201,208 ****
* Set up diagnostics.
*/
! if ((slash = strrchr(argv[0], '/')) != 0 && slash[1])
! tag = mail_task(slash + 1);
! else
! tag = mail_task(argv[0]);
if (isatty(STDERR_FILENO))
msg_vstream_init(tag, VSTREAM_ERR);
--- 200,204 ----
* Set up diagnostics.
*/
! tag = mail_task(argv[0]);
if (isatty(STDERR_FILENO))
msg_vstream_init(tag, VSTREAM_ERR);
***************
*** 217,224 ****
* Parse switches.
*/
while ((ch = GETOPT(argc, argv, "c:ip:t:v")) > 0) {
switch (ch) {
default:
! msg_fatal("usage: %s [-c config_dir] [-i] [-p priority] [-t tag]
[-v] [text]", tag);
break;
case 'c':
--- 213,221 ----
* Parse switches.
*/
+ tag = 0;
while ((ch = GETOPT(argc, argv, "c:ip:t:v")) > 0) {
switch (ch) {
default:
! msg_fatal("usage: %s [-c config_dir] [-i] [-p priority] [-t tag]
[-v] [text]", argv[0]);
break;
case 'c':
***************
*** 242,255 ****
/*
! * Process the main.cf file. This overrides any logging facility that was
! * specified with msg_syslog_init();
*/
mail_conf_read();
! if (tag == 0 && strcmp(var_syslog_name, DEF_SYSLOG_NAME) != 0) {
! if ((slash = strrchr(argv[0], '/')) != 0 && slash[1])
! tag = mail_task(slash + 1);
! else
! tag = mail_task(argv[0]);
! }
/*
--- 239,248 ----
/*
! * Process the main.cf file. This may change the syslog_name setting and
! * may require that mail_task() be re-evaluated.
*/
mail_conf_read();
! if (tag == 0)
! tag = mail_task(argv[0]);
/*
***************
*** 257,265 ****
* or on the command line.
*/
! if (tag != 0) {
! if (isatty(STDERR_FILENO))
! msg_vstream_init(tag, VSTREAM_ERR);
! msg_syslog_init(tag, LOG_PID, LOG_FACILITY);
! }
/*
--- 250,256 ----
* or on the command line.
*/
! if (isatty(STDERR_FILENO))
! msg_vstream_init(tag, VSTREAM_ERR);
! msg_syslog_init(tag, LOG_PID, LOG_FACILITY);
/*
