169536 - Unnecessary line of code

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Jörg Brunsmann]

The code shown at this line

http://lxr.mozilla.org/mozilla/source/intl/uconv/src/nsCharsetConverterManager.c
pp#125

is unnecessary

Comment 1

[Jörg Brunsmann]

... and also the cast at line

http://lxr.mozilla.org/mozilla/source/intl/uconv/src/nsCharsetConverterManager.c
pp#138

Comment 2

[Rui Xu]

code issue, QA to [e-mail] for now.

Comment 3

[Michael Lefevre]

code is still there, and from my limited knowledge of C++, doesn't look to have
changed, so I guess this should be confirmed...

Comment 4

[Frank Tang]

I think both roy and me are off mozilla for more than 2 years. If these bugs are
still here now, I think the real stauts is 'won't fix'. If you want to reopen
it, please find a new owner for it first. 

Comment 5

[Travis Chase]

Mass Reassign Please excuse the spam

Comment 6

[Travis Chase]

Mass Re-opening Bugs Frank Tang Closed on Wensday March 02 for no reason, all
the spam is his fault feel free to tar and feather him

Comment 7

[Travis Chase]

Reassigning Franks old bugs to Jungshik Shin for triage - Sorry for spam

Comment 8

[Simon Montagu :smontagu]

Attachment: Remove the unnecessary line (and some others)

Comment 9

[Simon Montagu :smontagu]

(In reply to comment #1)
> ... and also the cast at line
> 
> http://lxr.mozilla.org/mozilla/source/intl/uconv/src/nsCharsetConverterManager.c
> pp#138

This doesn't seem to exist any more.

Comment 10

[Jungshik Shin]

Comment on attachment 176453 [details] [diff] [review]
Remove the unnecessary line (and some others)

r=jshin
While you're at it, can you replace |res| with the standard |rv|?

Comment 11

[Simon Montagu :smontagu]

I'll also fix a typo s/nsResult/nsresult/ <blush>

Comment 12

[[e-mail]]

Comment on attachment 176453 [details] [diff] [review]
Remove the unnecessary line (and some others)

>   if (!aProp.IsEmpty()) key.Append(aProp.get()); // yes, this param may be NULL
While you're here can you fix this (twice) - aProp used to be a const
PRUnichar* which explains the null test but that .get() is also silly.

>   nsresult res = NS_OK;
> 
>   if (mTitleBundle == NULL) {
>     res = LoadExtensibleBundle(NS_TITLE_BUNDLE_CATEGORY, &mTitleBundle);
>     if (NS_FAILED(res)) return res;
>   }
This should use rv scoped inside the if (and the return on its own line too, as
you're touching it anyway).

Comment 13

[Simon Montagu :smontagu]

Fix checked in.