Issue 93461 - Fix the typo in the fix for database type detection.
Summary: Fix the typo in the fix for database type detection.
Status: CLOSED FIXED
Alias: None
Product: General
Classification: Code
Component: code (show other issues)
Version: current
Hardware: All All
: P3 Trivial (vote)
Target Milestone: OOo 3.0.1
Assignee: mikhail.voytenko
QA Contact: issues@framework
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-09-04 09:37 UTC by mikhail.voytenko
Modified: 2009-07-20 15:56 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description mikhail.voytenko 2008-09-04 09:37:19 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  :) "
Comment 1 mikhail.voytenko 2008-09-04 09:48:21 UTC
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.
Comment 2 Frank Schönheit 2008-09-04 09:55:30 UTC
> 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).
Comment 3 mikhail.voytenko 2008-09-04 10:17:23 UTC
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". :) 
Comment 4 Frank Schönheit 2008-09-04 10:19:48 UTC
ah - sorry, I got you wrong then :)
Comment 5 mikhail.voytenko 2008-09-11 10:55:25 UTC
Luckily, the typo had caused a warning in Linux x64 environment, so it was fixed
during integration. The assertion has been integrated.
Comment 6 mikhail.voytenko 2008-10-06 10:55:22 UTC
Development issue, setting to verified by myself.
Comment 7 thorsten.ziehm 2009-07-20 15:56:22 UTC
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