Apache OpenOffice (AOO) Bugzilla – Issue 93461
Fix the typo in the fix for database type detection.
Last modified: 2009-07-20 15:56:22 UTC
While fixing issue 93288 a typo was introduced. The comment from fs: "fs->mav: you were too generous in your fix: In revision 1.36.24.1 of dbloader.cxx, consider line 175 ff: if ( bStreamFromDescr && sURL.compareTo( ... ), 14 ) != COMPARE_EQUAL ); { ... } Note the superfluous semicolon at the end of the if-statement - it effectively means that the code inside the brackets, which is intended to be executed only if the if-statement evaluates to true, is executed unconditionally. I suppose it's not worth fixing this for OOO300, since it "just" means that the input-stream needs to be re-created when the document is actually loaded. But please fix it on the DEV300 branch. Also, another change which was not in the initial patch attached to this issue is the closing of the input stream. While this is a Good Thing (TM), please note that in DBA's code there's rarely such a thing as "catch( Exception& ) {}". This almost always has to be catch ( Exception& ) { DBG_UNHANDLED_EXCEPTION(); } , i.e. the error (and catching an exception here is an error) really should be reported in a non-product build, so it can at least be noticed and investigated, if it ever happens. (Oh, and one thing I in fact overlooked in the review of the attached patch is the minor thing that NamedValueCollection also has a ASCII version of "remove", so there's no need to create the ::rtl::OUString when calling it :) "
mav->fs: You are completely right, this typo should be fixed. Copy-paste approach is sometimes dangerous. I will also introduce the assertion for this case. Actually I had to ask you to review the final change as well. The last suggestion, as I understand, was intended to give me my small revenge ;) As far as I remember, it was suggested to use RTL_CONSTASCII_USTRINGPARAM() macro with ::rtl::OUString constructor instead of ::rtl::OUString::createFromAscii, since the last one has to measure the string length during the runtime.
> The last suggestion, as I understand, was intended to give me my small > revenge ;) :) Not all all, sorry if it looked that way. I just stumbled upon the "aMedia.remove( ::rtl::OUString(...) )", and I just wanted to mention (nothing more and nothing less) that there also is a version 'aMedia.remove( "<ascii_literal>" )', which is easier to type and easier to read (though it internally also creates an OUString).
mav->fs: By "give me my revenge" I meant "to give me a chance to prove that I am right at least in one point". :)
ah - sorry, I got you wrong then :)
Luckily, the typo had caused a warning in Linux x64 environment, so it was fixed during integration. The assertion has been integrated.
Development issue, setting to verified by myself.
This issue is closed automatically and wasn't rechecked in a current version of OOo. The fixed issue should be integrated in OOo since more than half a year. If you think this issue isn't fixed in a current version (OOo 3.1), please reopen it and change the field 'Target Milestone' accordingly. If you want to download a current version of OOo => http://download.openoffice.org/index.html If you want to know more about the handling of fixed/verified issues => http://wiki.services.openoffice.org/wiki/Handle_fixed_verified_issues