About last cppcheck fixes

[Noel Grandin]

Hi Julien

On 2015-02-17 08:32 AM, Julien Nabet wrote:
> Hello Noel,
>
> I noticed these lines in last patch about cppcheck fixes:
> diff --git a/basic/source/runtime/ddectrl.cxx b/basic/source/runtime/ddectrl.cxx
> index 2557c9e..232008a 100644
> --- a/basic/source/runtime/ddectrl.cxx
> <http://cgit.freedesktop.org/libreoffice/core/tree/basic/source/runtime/ddectrl.cxx?id=a30e2cb912c6bae240ced35a392151e34090665b>
> +++ b/basic/source/runtime/ddectrl.cxx
> <http://cgit.freedesktop.org/libreoffice/core/tree/basic/source/runtime/ddectrl.cxx?id=a8e6f0bea0e8bc028ee64d0b4d9046e52de94eda>
> @@ -133,7 +133,6 @@ SbError SbiDdeControl::Terminate( size_t nChannel )
> return SbERR_DDE_NO_CHANNEL;
> }
> delete pConv;
> - pConv = DDE_FREECHANNEL;
> return 0L;
> }
>
> Are you sure we can remove "pConv = DDE_FREECHANNEL;"? the test about if equal or different from DDE_FREECHANNEL seems
> used in the code.
>
> Indeed cppcheck returned this:
> [basic/source/runtime/ddectrl.cxx:136]: (style) Variable 'pConv' is assigned a value that is never used.
> but it seems a false positive since pConv is a pointer on a specific element of aConvList
>

pConv is a local variable, so the code that was removed was a dead assignment.

However, now that you mention it, it is possible that the method in question should actually look like:

     123 SbError SbiDdeControl::Terminate( size_t nChannel )
     124 {
     125     if (!nChannel || nChannel > aConvList.size())
     126     {
     127         return SbERR_DDE_NO_CHANNEL;
     128     }
     129     DdeConnection* pConv = aConvList[nChannel-1];
     130
     131     if( pConv == DDE_FREECHANNEL )
     132     {
     133         return SbERR_DDE_NO_CHANNEL;
     134     }
     135     delete pConv;
     136     aConvList[nChannel-1] = DDE_FREECHANNEL;  // <<<<<<<<<<<< fix
     137
     138     return 0L;
     139 }

CC'ing the list in case anyone knows better.

Regards, Noel.