Skip to content
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.

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

Always forward fputc to original fputc, with or without colors #54

Merged
merged 1 commit into from Jul 20, 2017

Conversation

DanOlivier
Copy link
Contributor

So LD_PRELOAD will affect every process forked from the console.
I hit an application that was affected by the fact that fputc was being silently redirected to fwrite (despite blacklisting).
The bug (see #53) was probably due to the differences in the meaning of the return codes.
To improve safety, this change (pull request) will not redirect the calls: fputc calls fputc, fwrite calls fwrite, and both will insert coloring commands before and after.

src/stderred.c Outdated
@@ -164,6 +194,14 @@ int FUNC(fprintf)(FILE *stream, const char *format, ...) {
return result;
}

int FUNC(__fprintf_chk)(FILE *fp, int flag, const char *format, ...) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function needs to be put behind a #ifdef check for existence since it is non standard.

See https://github.com/sickill/stderred/blob/master/src/CMakeLists.txt#L38-L50 for how

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do (soon)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@cehoffman
Copy link
Collaborator

Thank for this @DanOlivier, I think these are good changes.

@DanOlivier
Copy link
Contributor Author

Changes pertaining to __fprintf_chk are in pull #57.

@cehoffman cehoffman merged commit cb9c3fd into ku1ik:master Jul 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants