Date: prev next · Thread: first prev next last
2013 Archives by date, by thread · List index


On Sat, Aug 31, 2013 at 02:26:43PM -0700, julien2412 [via Document Foundation Mail Archive] wrote:

cppcheck reported this:
dbaccess/source/ui/browser/dbloader.cxx
167   redundantAssignment     style   Variable 'xNewKey' is reassigned a value
before the old one has been used

Here's the function:
    153 extern "C" void SAL_CALL writeDBLoaderInfo(void* pRegistryKey)
    154 {
    155     Reference< XRegistryKey> xKey(reinterpret_cast< XRegistryKey*>(pRegistryKey));
    156 
    157     // register content loader for dispatch
    158     OUString aImpl("/");
    159     aImpl += DBContentLoader::getImplementationName_Static();
    160 
    161     OUString aImpltwo = aImpl;
    162     aImpltwo += "/UNO/Loader";
    163     Reference< XRegistryKey> xNewKey = xKey->createKey( aImpltwo );
    164     aImpltwo = aImpl;
    165     aImpltwo += "/Loader";
    166     Reference< XRegistryKey >  xLoaderKey = xKey->createKey( aImpltwo );
    167     xNewKey = xLoaderKey->createKey( OUString("Pattern") );
    168     xNewKey->setAsciiValue( OUString(".component:DB*") );
    169 }
(see
http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/browser/dbloader.cxx#153)

Indeed, xNewKey is reassigned. But now I don't know how it should be
changed.

The general "mechanical" rule is:

 - if the function whose return value populates the variable has a
   side effect, then just remove the
   Reference< XRegistryKey> xNewKey =

 - else, remove the whole line (and in this case, the string
   construction that prepares the argument to the function call).

"Side effect" means that the function has another function than
constructing its return value; that is, it changes the state of the
program in a way that after the return value is destroyed, still has
an effect.

In this particular case, it seems to me that it has a side effect,
namely creating the said configuration / registry key: if some other
code subsequently calls "openKey", they will get non-NULL, whereas
without that createKey() they will get NULL. The structure of the code
(with a aImpl and aImpltwo that gets reused) also looks to me like
this is done on purpose (as opposed to a copy/paste error).

On the other hand, it seems unusual to create an "empty" key just so
that future code will find it... And I don't find any other instance
of the string "UNO/Loader" anywhere in the code. Based on this
observation, I would remove the whole "createKey".

On the gripping hand, I can't find where the "//Loader" key is used
afterwards either, so maybe my search-fu simply failed me. Or maybe
the whole function is pointless. As an added bonus remark, it seems
weird (but maybe innocuous) that it goes to extra pains to create
"//UNO/Loader" and "//Loader" as opposed to "/UNO/Loader" and
"/Loader".

And searching more, I see that this function is actually never
called... It is not in any header file of the sdk (not in _any_ header
actually), so it can't be part of our public API (nor of our internal
API).

So actually, in this case, remove the whole function. And also the
function of the same name in dbaccess/source/filter/xml/dbloader2.cxx

The commit that removed its usage is:

commit 8e88ac109dc9eba88db92940d13933fc3a4393d8
Author: sb <sb@openoffice.org>
Date:   Fri Sep 10 13:10:07 2010 +0200

    sb129: #i113189# change UNO components to use passive registration

-- 
Lionel




--
View this message in context: 
http://nabble.documentfoundation.org/About-writeDBLoaderInfo-function-in-dbloader-cxx-dbaccess-module-tp4072511p4072521.html
Sent from the Dev mailing list archive at Nabble.com.

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.