After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 754698 - g++-5: newsrc-* files are created in current working directory instead of ~/.pan due to change in std::string return semantics
g++-5: newsrc-* files are created in current working directory instead of ~/....
Status: RESOLVED FIXED
Product: Pan
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: pan-maint
pan-maint
Depends on:
Blocks:
 
 
Reported: 2015-09-07 17:19 UTC by Alexandre Rostovtsev
Modified: 2016-03-06 00:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (1.20 KB, patch)
2015-09-07 17:23 UTC, Alexandre Rostovtsev
committed Details | Review

Description Alexandre Rostovtsev 2015-09-07 17:19:20 UTC
g++-5 abandons the non-standard CoW semantics for std::string, instead using interning for short strings. As a result, code like this no longer works:

    std::string file::get_pan_home() {...}
    const char * ph = file::get_pan_home().c_str();

because that c_str() returns start of buffer of a temporary std::string which is deleted at the semicolon - and ph is now pointing into the neverland.

Of course, that sort of code had never worked with other c++ compilers.

In general, returning std::string objects is bad practice - both because of unnecessary memory copying and heap fragmentation, and because it's tempting to call methods on those objects when they are still ephemeral, leading to crashes and bugs.

The end result of all this is some of pan's files like newrsc-1 etc. are being created in the current working directory instead of ~/.pan - since with g++-5, file::absolute_fn() in git master and newsrc_fn() in 0.139 tarball release are broken. This was reported to us downstream at https://bugs.gentoo.org/show_bug.cgi?id=559666

The attached patch fixes file::get_pan_home() by returning const string reference to the static function-local object instead of a new string object.

My suggestion is to carefully go through the entire project codebase - there are lots of functions returning std::string, and I suspect that not all of their callers are compatible with the standard, non-g++-4 string semantics.
Comment 1 Alexandre Rostovtsev 2015-09-07 17:23:00 UTC
Created attachment 310852 [details] [review]
proposed patch
Comment 2 Petr Kovar 2015-12-20 19:46:40 UTC
Review of attachment 310852 [details] [review]:

Many thanks for providing the patch.

I've pushed it to master as:
https://git.gnome.org/browse/pan2/commit/?id=34e1bb5f76bc0026091a6f494c354b0064e8ace0
Comment 3 Petr Kovar 2015-12-20 19:57:34 UTC
(In reply to Alexandre Rostovtsev from comment #0)

> My suggestion is to carefully go through the entire project codebase - there
> are lots of functions returning std::string, and I suspect that not all of
> their callers are compatible with the standard, non-g++-4 string semantics.

Good suggestion, though, as you can see from pan's git log, pan is not being actively maintained atm, so I'm afraid until the project finds a developer willing to go through the code and fix it, this problem will remain unresolved.

I'll leave this bug open for now but can't do much more than suggest distributors start shipping the latest git snapshot instead of the very outdated 0.139. The latest code might contain some additional fixes, though probably unrelated.

Thanks again.
Comment 4 Petr Kovar 2016-03-06 00:01:19 UTC
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.

Please reopen if the problem persists, thank you.