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

DM-7509: Fix two bugs in handling strings with %s #14

Merged
merged 4 commits into from Sep 3, 2016
Merged

Conversation

timj
Copy link
Member

@timj timj commented Sep 2, 2016

The bugs are:

  • If one logs a message with a string that contains %s
    and no format arguments, an exception is raised
    because % formatting is attempted
  • If one logs a message with a string that contains %s
    after formatting with data arguments that are provided
    the system segfaults because Log.log is called
    (which attempts formatting) instead of Log.logMsg

inspect.stack()[2][3], frame.f_lineno, fmt % args)
msg = fmt % args if args else fmt
self.logMsg(level, os.path.split(frame.f_code.co_filename)[1],
inspect.stack()[2][3], frame.f_lineno, fmt % msg)
Copy link
Member Author

Choose a reason for hiding this comment

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

do you mean fmt % msg here? Not just msg?

@r-owen r-owen force-pushed the tickets/DM-7509 branch 2 times, most recently from d71ed6b to 3ec0223 Compare September 3, 2016 13:47
inspect.stack()[2][3], frame.f_lineno, fmt % args)
msg = fmt % args if args else fmt
self.logMsg(level, os.path.split(frame.f_code.co_filename)[1],
inspect.stack()[2][3], frame.f_lineno, msg)
Copy link
Member Author

Choose a reason for hiding this comment

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

indenting doesn't match the open paren.

@timj
Copy link
Member Author

timj commented Sep 3, 2016

tests/testLog has been committed by mistake.

@@ -61,10 +61,10 @@ import os
%include "lsst/log/Log.h"

%extend lsst::log::Log {
void log(int level, std::string const& filename,
void logMsg(int level, std::string const& filename,
std::string const& funcname, unsigned int lineno,
Copy link
Member Author

Choose a reason for hiding this comment

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

indenting.

The bugs are:
- If one logs a message with a string that contains %s
  and no format arguments, an exception is raised
  because % formatting is attempted
- If one logs a message with a string that contains %s
  after formatting with data arguments that are provided
  the system segfaults because Log.log is called
  (which attempts formatting) instead of Log.logMsg
Also remove obsolete tests entry from .gitignore
@r-owen
Copy link
Contributor

r-owen commented Sep 3, 2016

I fixed the indentation issues noted by @timj

@r-owen r-owen merged commit e6c6b0a into master Sep 3, 2016
@ktlim ktlim deleted the tickets/DM-7509 branch August 25, 2018 05:18
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

3 participants