Re: About last cppcheck fixes

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.