Skip to content

Fix for #69061 - Remove timestamp for mail.log = syslog #1087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
tomsommer wants to merge 6 commits into from
Closed

Fix for #69061 - Remove timestamp for mail.log = syslog #1087

tomsommer wants to merge 6 commits into from

Conversation

@tomsommer
Copy link
Contributor

@tomsommer tomsommer commented Feb 16, 2015 •
edited
Loading

Fix for #69061

  • Remove timestamp when sending to syslog
  • Make mail.log append correct PHP_EOL to file

@jpauli jpauli added the Bug label Feb 19, 2015
smalyshev
smalyshev reviewed Mar 23, 2015
View reviewed changes
ext/standard/mail.c Outdated
Copy link
Contributor

@smalyshev smalyshev Mar 23, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will leak the old buffer occupied by msg, since spprintf will allocate the new one and the pointer to the old one will be lost. Please fix.

tomsommer added 3 commits July 14, 2015 09:26
@tomsommer
Update mail.c
2e0ddf5 
Fix for #69061

Make mail.log append correct PHP_EOL and remove timestamp when sending to syslog
@tomsommer
Update mail.c
1e22991
@tomsommer
Fix leak
f40f2e5
cmb69
cmb69 reviewed Aug 6, 2015
View reviewed changes
ext/standard/mail.c Outdated
Copy link
Member

@cmb69 cmb69 Aug 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't date_str already been freed in line 299?

Copy link
Contributor Author

@tomsommer tomsommer Aug 6, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@krakjoe
Copy link
Member

krakjoe commented Jan 2, 2017

@tomsommer fix conflicts please

@krakjoe
Copy link
Member

krakjoe commented Jan 13, 2017

For some reason I'm still not able to merge this, even though there are no conflicts reported ... possibly your master branch is out of date ?

@cmb69
Copy link
Member

cmb69 commented Jan 13, 2017

@krakjoe Possibly related to http://news.php.net/php.internals/97726.

@nikic nikic self-assigned this Jan 14, 2017
@nikic
Copy link
Member

nikic commented Jan 14, 2017

Merged via e77a1db into 7.1+, thanks! I didn't merge into 7.0 as I don't want to change log file format in the middle of the release cycle (could break parsers etc).

@krakjoe Not sure what the issue was, I just applied the diff directly. We should probably ask people to rebase changes onto master rather than merging it in. Otherwise the git am workflow doesn't really work and we either have to merge (which precludes merging to lower branches ... you know best what happens in that case) or use git apply with git commit --author and copying in a commit message.

@nikic nikic closed this Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nikic nikic

Labels

Bug Waiting on Author

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tomsommer @krakjoe @cmb69 @nikic @smalyshev @jpauli