Skip to content

[Math] GSLMultiRootFinder: Fix return pointer to local address #613

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

Closed
amadio wants to merge 1 commit into from
Closed

[Math] GSLMultiRootFinder: Fix return pointer to local address #613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
[Math] GSLMultiRootFinder: Fix return pointer to local address
  • Loading branch information
@amadio
amadio committed Jun 7, 2017
commit e00cc61a1c11ecc3261a185ac603f40bb58614cf
6 changes: 3 additions & 3 deletions math/mathmore/inc/Math/GSLMultiRootFinder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,9 @@ namespace Math {
/// Return the status of last root finding
int Status() const { return fStatus; }

/// Return the algorithm name used for solving
/// Note the name is available only after having called solved
/// Otherwise an empyty string is returned
/// Return the name of the algorithm used for solving
/// Note that the name is only available after Solve()
/// has been called. Returns nullptr if called before Solve().
const char * Name() const;

/*
Expand Down
2 changes: 1 addition & 1 deletion math/mathmore/src/GSLMultiRootFinder.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ const double * GSLMultiRootFinder::FVal() const {
}
const char * GSLMultiRootFinder::Name() const {
// get GSL name
return (fSolver != 0) ? fSolver->Name().c_str() : "";
return (fSolver != 0) ? fSolver->Name().c_str() : nullptr;
Copy link
Member

@Axel-Naumann Axel-Naumann Jun 7, 2017

Choose a reason for hiding this comment

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

That should not be needed? That address actually has static storage duration as per [lex.string]. Are you sure you're solving a problem in the current code version? (Note that we had a problem here because fSolver->Name() returned a std::string; it now returns a std::string&.)

Copy link
Member Author

@amadio amadio Jun 7, 2017

Choose a reason for hiding this comment

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

The compiler is complaining about it, so it could be a problem. I just noticed that the problem is not about the empty string, though, but about fSolver->Name().c_str(). So, in hindsight, while my "fix" is probably pointless, it does seem like the issue with returning a local address still remains.

Copy link
Member

@lmoneta lmoneta Jun 7, 2017

Choose a reason for hiding this comment

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

The problem of fSolver->Name()->c_str() should have been fixed by my commit of yesterday.
Strange the compiler is still complaining. Which version ?

Copy link
Member Author

@amadio amadio Jun 7, 2017

Choose a reason for hiding this comment

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

It's GCC 7 complaining. The warning in CDash can be found here.

}

// bool GSLMultiRootFinder::AddFunction( const ROOT::Math::IMultiGenFunction & func) {
Expand Down