Bug 94879 - add 'm' prefixes to member variables ...
Summary: add 'm' prefixes to member variables ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.0.2.1 rc
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.1.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2015-10-08 11:57 UTC by Michael Meeks
Modified: 2016-10-25 19:19 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2015-10-08 11:57:30 UTC
The Storage code is currently really hard to read - since its missing 'm' suffixes on its variables:

sot/source/sdstor.cxx:

bool StgStrm::Pos2Page( sal_Int32 nBytePos )

    if ( !pFat )
        return false;

    // Values < 0 seek to the end
    if( nBytePos < 0 || nBytePos >= nSize )
        nBytePos = nSize;
    // Adjust the position back to offset 0
    nPos -= nOffset;
    sal_Int32 nMask = ~( nPageSize - 1 );
    sal_Int32 nOld = nPos & nMask;
    sal_Int32 nNew = nBytePos & nMask;
    nOffset = (short) ( nBytePos & ~nMask );
...

Pretty quickly the 'nPos' (a member) gets mixed with the 'nNew' (a local) etc.

The usual convention is to use an 'm' prefix; so: 'mpFat' 'mnPos' 'mnOffset' here to make this much more readable.

It'd be great to have a patch that adds that for members in this module; perhaps separate patches for similar problems in other sot/ modules would be good too.

Thanks !
Comment 1 Miklos Vajna 2015-10-08 12:24:33 UTC
Before it would be done manually, I recently wrote a tool based on clang libtooling to automate this process.

http://cgit.freedesktop.org/libreoffice/contrib/dev-tools/tree/clang has the code and documentation on how to do it, basically in this case (because this class is internal to the sot module) it's a matter of running:

make -sr COMPILER_EXTERNAL_TOOL=1 CCACHE_PREFIX=find-unprefixed-members-wrapper RENAME_ARGS="-class-name=StgStrm"

to generate a .csv file, then:

make -sr COMPILER_EXTERNAL_TOOL=1 CCACHE_PREFIX=rename-wrapper RENAME_ARGS="-csv=$HOME/rename.csv"

to generate .new files where the members are prefixed, finally:

for i in $(find . -name "*.new"); do mv -f $i ${i%%.new}; done

to overwrite the original files with the .new ones. All these commands have to be invoked in the sot/ directory of a build tree where clang is the configured compiler.
Comment 2 Commit Notification 2015-10-10 18:30:50 UTC
melikeyurtoglu committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=30b3676624685e260e2ec1909486b971fac129f7

tdf#94879 add 'm' suffixes to member variables

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 3 Miklos Vajna 2015-11-27 10:26:42 UTC
Forgot to mark this one as resolved.
Comment 4 Robinson Tryon (qubit) 2015-12-16 00:10:34 UTC Comment hidden (noise)