> On May 4, 2016, 11:20 a.m., Friedrich W. H. Kossebau wrote: > > src/lib/kaboutdata.cpp, line 925 > > <https://git.reviewboard.kde.org/r/127655/diff/2/?file=462409#file462409line925> > > > > Forgot to note the static I am used to write here. But given your > > comment I am now unsure about my C++ foo: checked the lib binary with nm, > > and no matter whether I used static, namespace {} (& inline just for the > > sake) or a combination of them, the method was always listed as a private > > symbol (nm | grep): > > 000000000002486f t _ZL15warnIfOutOfSyncPKcRK7QStringS0_S3_ > > 0000000000081900 r > > _ZZL15warnIfOutOfSyncPKcRK7QStringS0_S3_E19__PRETTY_FUNCTION__ > > > > Which also matches what I would have expected before your comment, so > > need your handholding what and how this can be improved here exactly :)
I think private symbols are just an anomaly of the way the toolchain handles compiling/linking in face of possibility that debugging/LTO/etc. might happen. The important thing is that they are not listed as public symbols, they will compile with more optimizations/inlining (since they can't be interposed using LD_PRELOAD), etc. So either static or anon. namespace is fine. There's some reason to prefer anon. namespace that I forget offhand but either is fine here. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127655/#review95158 ----------------------------------------------------------- On May 4, 2016, 2:47 p.m., Friedrich W. H. Kossebau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127655/ > ----------------------------------------------------------- > > (Updated May 4, 2016, 2:47 p.m.) > > > Review request for KDE Frameworks, Alex Richardson and Michael Pyne. > > > Repository: kcoreaddons > > > Description > ------- > > There is code in e.g. KXMLGUI which relies on KAboutData::applicationData(), > without requiring the user to use KAboutData::setApplicationData(). So better > be complete when initializing the data from the Q*Application metadata. > > Also > - warn if there is no Q*App instance yet to set properties in > KAboutData::setApplicationData > - check and warn if KAboutData::applicationData is out-of-sync with qapp data > - remove bogus check for empty display name, as the method defaults to > componentname > - unit tests > KAboutDataTest::testSetOfQApplicationData/testPickupOfQApplicationData > > > Diffs > ----- > > autotests/CMakeLists.txt a7a6752 > autotests/kaboutdataapplicationdatatest.cpp PRE-CREATION > src/lib/kaboutdata.h 97c0f2b > src/lib/kaboutdata.cpp ceb0c06 > > Diff: https://git.reviewboard.kde.org/r/127655/diff/ > > > Testing > ------- > > Added autotests pass. > > > Thanks, > > Friedrich W. H. Kossebau > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
