Cppcheck reports "reassign before using" variable in sw/source/core/doc/tblafmt.cxx

[Michael Stahl]

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.