Seems the only user of it is sal/rtl/source/bootstrap.cxx it could easily be replaced with something like: oslProfile profile = osl_openProfile(strProfileName.pData, Options); if( ! profile ) throw std::exception(); sal_Char aBuf[1024]; rtl::OString ret= osl_readProfileString( profile, rSection.getStr(), rEntry.getStr(), aBuf, sizeof( aBuf ), rDefault.getStr() ) ? rtl::OString( aBuf ) : rtl::OString(); osl_closeProfile(profile); return ret; and then deleted from codebase.
is this code cleanup still actual?
(In reply to comment #0) > Seems the only user of it is sal/rtl/source/bootstrap.cxx > > it could easily be replaced with something like: > > oslProfile profile = osl_openProfile(strProfileName.pData, Options); > if( ! profile ) > throw std::exception(); > > sal_Char aBuf[1024]; > rtl::OString ret= osl_readProfileString( profile, rSection.getStr(), > rEntry.getStr(), aBuf, sizeof( aBuf ), rDefault.getStr() ) ? rtl::OString( > aBuf ) : rtl::OString(); > > osl_closeProfile(profile); > > return ret; Given the comment preceding the (only) use of osl::Profile in sal/rtl/bootstrap.cxx, // Going through osl::Profile, this code erroneously // does not recursively expand macros in the resulting // replacement text (and if it did, it would fail to // detect cycles that pass through here): it does not make much sense to merely unwrap that code from using the C++ osl/profile.hxx wrapper to using the underlying C osl/profile.h functionality. Also note that this is part of the stable URE interface, so cannot be modified arbitrarily (but can arguably be removed in an incompatible release, as it is deprecated for a long time and should effectively be unused by client code). So, a useful approach could be: * Adapt sal/rtl/bootstrap.cxx to no longer use the OSL Profile functionality, and in doing so fix the bug described in the comment above. * In a commit with "[API CHANGE]" in the commit message, remove the include/osl/profile.hxx wrapper. * In the same or a subsequent "[API CHANGE]" commit, also remove include/sal/profile.h and replace the implementation with stubs in sal/osl/all/compat.cxx (which are necessary as the corresponding symbols could not be removed from sal/util/sal.map without changing the sal SONAME, which we do not want to do). * Document the incompatible changes in the "API Changes" section of the relevant <https://wiki.documentfoundation.org/ReleaseNotes>.
** Please read this message in its entirety before responding ** To make sure we're focusing on the bugs that affect our users today, LibreOffice QA is asking bug reporters and confirmers to retest open, confirmed bugs which have not been touched for over a year. There have been thousands of bug fixes and commits since anyone checked on this bug report. During that time, it's possible that the bug has been fixed, or the details of the problem have changed. We'd really appreciate your help in getting confirmation that the bug is still present. If you have time, please do the following: *Test to see if the bug is still present on a currently supported version of LibreOffice (4.4.1 or later) https://www.libreoffice.org/download/ *If the bug is present, please leave a comment that includes the version of LibreOffice and your operating system, and any changes you see in the bug behavior *If the bug is NOT present, please set the bug's Status field to RESOLVED-WORKSFORME and leave a short comment that includes your version of LibreOffice and Operating System Please DO NOT *Update the version field *Reply via email (please reply directly on the bug tracker) *Set the bug's Status field to RESOLVED - FIXED (this status has a particular meaning that is not appropriate in this case) If you want to do more to help you can test to see if your issue is a REGRESSION. To do so: 1. Download and install oldest version of LibreOffice (usually 3.3 unless your bug pertains to a feature added after 3.3) http://downloadarchive.documentfoundation.org/libreoffice/old/ 2. Test your bug 3. Leave a comment with your results. 4a. If the bug was present with 3.3 - set version to "inherited from OOo"; 4b. If the bug was not present in 3.3 - add "regression" to keyword Feel free to come ask questions or to say hello in our QA chat: http://webchat.freenode.net/?channels=libreoffice-qa Thank you for your help! -- The LibreOffice QA Team This NEW Message was generated on: 2015-04-01
the issue is still present in the latest GIT
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyInteresting SkillCpp TopicCleanup) [NinjaEdit]
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
A polite ping, still working on this issue?
Unassigning due to lack of work.
Re-evaluating the EasyHack in 2022 This enhancement is still relevant. One should read comment 2 and follow the steps written by Stephan.
In order to discard the use of deprecated osl::profile and fix the bug mentioned in the comments, do I have to use expandmacros function?
(In reply to seturajmatroja from comment #10) > In order to discard the use of deprecated osl::profile and fix the bug > mentioned in the comments, do I have to use expandmacros function? yes, some recursive use of that macro expansion functionality, more or less
Are there any previous api changes regarding osl::profile so I can refer them? Also there is a recursive expandmacros function in the file
(In reply to seturajmatroja from comment #12) > Are there any previous api changes regarding osl::profile so I can refer > them? no, none that I'm aware of > Also there is a recursive expandmacros function in the file yes, and it might be part of the fix here, but that needs looking into (which is the easy hack task to be done here :) that's why I wrote "more or less"
Does it even reach that point? As when I tried to run it in full debug mode and added breakpoints to the profile , it never hit that line.
(In reply to seturajmatroja from comment #14) > Does it even reach that point? The relevant code in expandMacros would be reached for a reference of the form > ${<absolute-ini-file-URI>:<section>:<key>} which is likely not encountered in a typical LO installation set. (So you would likely set up some test code for it. There already is e.g. sal/qa/rtl/bootstrap/expand.cxx.) Caveat: The existing rtl/bootstrap code just effectively ignores "[section]" markers in the ini-files it processes. It uniformly consumes all key=value pairs regardless of what sections, if any, they are in. (It nominally requires that bootstrap variables are in "[bootstrap]" sections, but does not enforce that. And, for backwards compatibility, we should likely continue with this behavior.) So there would be a problem how to treat the <section> part in the above reference, if we were to change the processing code from osl/profile to existing rtl/bootsrap functionality. The change proposed here might or might not be such a bright idea after all, and might or might not actually be an easy hack (though it already has the high category "interesting", for a reason).