On Sat, Aug 31, 2013 at 02:26:43PM -0700, julien2412 [via Document Foundation Mail Archive] wrote:
<br/><br/>> cppcheck reported this:
<br/>> dbaccess/source/ui/browser/dbloader.cxx
<br/>> 167     redundantAssignment     style   Variable 'xNewKey' is reassigned a value
<br/>> before the old one has been used
<br/><div class='shrinkable-quote'><br/>> Here's the function:
<br/>>     153 extern "C" void SAL_CALL writeDBLoaderInfo(void* pRegistryKey)
<br/>>     154 {
<br/>>     155     Reference< XRegistryKey> xKey(reinterpret_cast< XRegistryKey*>(pRegistryKey));
<br/>>     156 
<br/>>     157     // register content loader for dispatch
<br/>>     158     OUString aImpl("/");
<br/>>     159     aImpl += DBContentLoader::getImplementationName_Static();
<br/>>     160 
<br/>>     161     OUString aImpltwo = aImpl;
<br/>>     162     aImpltwo += "/UNO/Loader";
<br/>>     163     Reference< XRegistryKey> xNewKey = xKey->createKey( aImpltwo );
<br/>>     164     aImpltwo = aImpl;
<br/>>     165     aImpltwo += "/Loader";
<br/>>     166     Reference< XRegistryKey >  xLoaderKey = xKey->createKey( aImpltwo );
<br/>>     167     xNewKey = xLoaderKey->createKey( OUString("Pattern") );
<br/>>     168     xNewKey->setAsciiValue( OUString(".component:DB*") );
<br/>>     169 }
<br/>> (see
<br/>> <a href="http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/browser/dbloader.cxx#153" target="_top" rel="nofollow" link="external">http://opengrok.libreoffice.org/xref/core/dbaccess/source/ui/browser/dbloader.cxx#153</a>)
</div><br/>> Indeed, xNewKey is reassigned. But now I don't know how it should be
<br/>> changed.
<br/><br/>The general "mechanical" rule is:
<br/><br/> - if the function whose return value populates the variable has a
<br/>   side effect, then just remove the
<br/>   Reference< XRegistryKey> xNewKey =
<br/><br/> - else, remove the whole line (and in this case, the string
<br/>   construction that prepares the argument to the function call).
<br/><br/>"Side effect" means that the function has another function than
<br/>constructing its return value; that is, it changes the state of the
<br/>program in a way that after the return value is destroyed, still has
<br/>an effect.
<br/><br/>In this particular case, it seems to me that it has a side effect,
<br/>namely creating the said configuration / registry key: if some other
<br/>code subsequently calls "openKey", they will get non-NULL, whereas
<br/>without that createKey() they will get NULL. The structure of the code
<br/>(with a aImpl and aImpltwo that gets reused) also looks to me like
<br/>this is done on purpose (as opposed to a copy/paste error).
<br/><br/>On the other hand, it seems unusual to create an "empty" key just so
<br/>that future code will find it... And I don't find any other instance
<br/>of the string "UNO/Loader" anywhere in the code. Based on this
<br/>observation, I would remove the whole "createKey".
<br/><br/>On the gripping hand, I can't find where the "//Loader" key is used
<br/>afterwards either, so maybe my search-fu simply failed me. Or maybe
<br/>the whole function is pointless. As an added bonus remark, it seems
<br/>weird (but maybe innocuous) that it goes to extra pains to create
<br/>"//UNO/Loader" and "//Loader" as opposed to "/UNO/Loader" and
<br/>"/Loader".
<br/><br/>And searching more, I see that this function is actually never
<br/>called... It is not in any header file of the sdk (not in _any_ header
<br/>actually), so it can't be part of our public API (nor of our internal
<br/>API).
<br/><br/>So actually, in this case, remove the whole function. And also the
<br/>function of the same name in dbaccess/source/filter/xml/dbloader2.cxx
<br/><br/>The commit that removed its usage is:
<br/><br/>commit 8e88ac109dc9eba88db92940d13933fc3a4393d8
<br/>Author: sb <<a href="/user/SendEmail.jtp?type=node&node=4072521&i=0" target="_top" rel="nofollow" link="external">[hidden email]</a>>
<br/>Date:   Fri Sep 10 13:10:07 2010 +0200
<br/><br/>    sb129: #i113189# change UNO components to use passive registration
<br/><br/>-- 
<br/>Lionel
<br/>

        
        
        
<br/><hr align="left" width="300" />
View this message in context: <a href="http://nabble.documentfoundation.org/About-writeDBLoaderInfo-function-in-dbloader-cxx-dbaccess-module-tp4072511p4072521.html">Re: About writeDBLoaderInfo function in dbloader.cxx (dbaccess module)</a><br/>
Sent from the <a href="http://nabble.documentfoundation.org/Dev-f1639786.html">Dev mailing list archive</a> at Nabble.com.<br/>