On 20/12/12 00:26, julien2412 wrote:
Hello,
Cppcheck detected this:
[sw/source/core/doc/tblafmt.cxx:1227] ->
[sw/source/core/doc/tblafmt.cxx:1234]: (performance) Variable 'bRet' is
reassigned a value before the old one has been used.
1221 // Attention: We need to save a general Header here
1222 sal_uInt16 nVal = AUTOFORMAT_ID;
1223 rStream << nVal
1224 << (sal_uInt8)2 // Character count of the Header
including this value
1225 << (sal_uInt8)GetStoreCharSet(
::osl_getThreadTextEncoding() );
1226
1227 bRet = 0 == rStream.GetError();
1228
1229 // Write this version number for all attributes
1230 m_pImpl->m_AutoFormats[0].GetBoxFmt(0).SaveVersionNo(
1231 rStream, AUTOFORMAT_FILE_VERSION);
1232
1233 rStream <<
static_cast<sal_uInt16>(m_pImpl->m_AutoFormats.size() - 1);
1234 bRet = 0 == rStream.GetError();
see
http://opengrok.libreoffice.org/xref/core/sw/source/core/doc/tblafmt.cxx#1215
What's the use of line 1227 if bRet isn't used before its new assignation
line 1234?
Should this line be removed because we're sure lines before will be ok or
should something be added here?
the error handling there leaves a lot to be desired...
ideally every assignment to bRet should be followed by a test "if
(!bRet) return false;", i don't think it's a good idea to try to
continue storing the file if something failed and then end up with a
corrupt file. there are apparently other functions in that cxx file
with similar problems.
Context
Privacy Policy |
Impressum (Legal Info) |
Copyright information: Unless otherwise specified, all text and images
on this website are licensed under the
Creative Commons Attribution-Share Alike 3.0 License.
This does not include the source code of LibreOffice, which is
licensed under the Mozilla Public License (
MPLv2).
"LibreOffice" and "The Document Foundation" are
registered trademarks of their corresponding registered owners or are
in actual use as trademarks in one or more countries. Their respective
logos and icons are also subject to international copyright laws. Use
thereof is explained in our
trademark policy.