184075 - nsAboutRedirector should not use literal number

Status: VERIFIED FIXED

(Core :: Networking, defect, P2)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

currently, AboutRedirector has a line like this:
static const int kRedirTotal = 4; // sizeof(kRedirMap)/sizeof(*kRedirMap)

instead, it should use the piece of code that is commented out... unless there
is a reason not to do that?

I'll attach a patch in a second; darin/bz, please review or tell me why it's a
bad idea.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 2

[Darin Fisher]

Comment on attachment 108584 [details] [diff] [review]
patch

cvs blame says you checked in the patch that commented out that line of code
(see bug 127252).  what were your reasons for doing so?  off hand, your patch
here seems valid, but can you please test it to make sure that it works as
expected?

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

er, no... my patch did this:
-static const int kRedirTotal = 3; // sizeof(kRedirMap)/sizeof(*kRedirMap)
+static const int kRedirTotal = 4; // sizeof(kRedirMap)/sizeof(*kRedirMap)
(see http://bugzilla.mozilla.org/attachment.cgi?id=70920&action=view)

I just adjusted the number, not changed the comment

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 108584 [details] [diff] [review]
patch

mmm... non-fragile code... ;)

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

sure, I tested it, and it works :)

Comment 6

[Darin Fisher]

biesi: sorry, i should have checked more carefully at the CVS history :-(

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

fixed

Comment 8

[benc]

VERIFIED: lxr.
I had to do some URL hacking in CVS history to get the view that showed the
[e-mail] changes to that line...