243429 - XPCOM_BREAK_ON_LOAD no longer works

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Brian Ryner (not reading)]

After the string branch landing, XPCOM_BREAK_ON_LOAD no longer works due to a
subtle behavior change in nsString::Find() when the starting index is < 0.

Comment 1

[Brian Ryner (not reading)]

Attachment: patch

Restore the old behavior -- a starting index < 0 is treated as starting from
the beginning of the string.

Comment 2

[Darin Fisher]

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

thanks bryner!

Comment 3

[Brian Ryner (not reading)]

checked in

Comment 4

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

*** Bug 243095 has been marked as a duplicate of this bug. ***

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Despite this fix, XPCOM_BREAK_ON_LOAD is still busted in RFind on MinGW.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Er, I meant Find.

Comment 7

[Colin Blake]

Yep, busted on OpenVMS too (see my comment today in bug 240837).

Comment 8

[Colin Blake]

My mistake. The fix does fix it. Its just that the fix isn't in 1.7. Sigh.....

Comment 9

[Darin Fisher]

dmose: what build were you referring to in comment #5?  a 1.7 branch build /
aviary 1.0 build?  if so, can we close this as fixed again?  colin is saying
that this is only fixed on the trunk.

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Darin: this happens on both the trunk and 1.7 branch for me.

Comment 11

[Mike Kaply [:mkaply]]

Was this put in aviary?

I think I'd like it in 1.7 also.

Comment 12

[Dan Mosedale (:dmosedale, :dmose)]

Last I checked, this was still broken on MinGW, and Mike says he didn't check in
any changes on the trunk.

Comment 13

[YAMASHITA Makoto]

Attachment: modernize modulename matching

Does this (+ "suspend" for XPCOM_DEBUG_BREAK) work for you? I need these to have break-on-load work on Mac OS X (10.3.9).

Comment 14

[Darin Fisher]

> Created an attachment (id=207049) [edit]
> modernize modulename matching

NOTE: The old code attempted case-insensitive substring matching.  This patch changes it to case-sensitive substring matching.  Is that really desirable?

Comment 15

[YAMASHITA Makoto]

Attachment: case insensitive modulename search

how about this one?

Comment 16

[YAMASHITA Makoto]

Attachment: recover multiple module names search

The prev patch broke colon-separated multiple module names search.
Curiously now I don't need XPCOM_DEBUG_BREAK again....

Comment 17

[Brian Ryner (not reading)]

Comment on attachment 207411 [details] [diff] [review]
recover multiple module names search

Can you ask Darin instead?

Comment 18

[Darin Fisher]

Comment on attachment 207411 [details] [diff] [review]
recover multiple module names search

>Index: nsNativeComponentLoader.cpp

>-            if (leafName.Find(token, PR_TRUE)) {
>+            nsDependentCString tokCStr(token);
>+            nsACString::const_iterator start, end;
>+            leafName.BeginReading(start);
>+            leafName.EndReading(end);
>+            if (CaseInsensitiveFindInReadable(tokCStr, start, end)) {
>                 NS_BREAK();

I think the old bug was that the return value of Find was not
checked correctly.  This should work too:

              if (leafName.Find(token, PR_TRUE) != kNotFound) {
                  NS_BREAK();
              }

If that works, then I would use that instead as it is a lot less
code.

Comment 19

[YAMASHITA Makoto]

Attachment: compare to kNotFound

I think this is a good chance to get rid of the obsolete api, but I don't mind if you prefer this one.

Comment 20

[YAMASHITA Makoto]

Attachment: compare to kNotFound

I think this is a good chance to get rid of the obsolete api, but I don't mind if you prefer this one.

Comment 21

[Darin Fisher]

Comment on attachment 208188 [details] [diff] [review]
compare to kNotFound

r+sr=darin

I think there are parts of the "obsolete" string API such as Find and RFind that are in fact better than the functions in nsReadableUtils.h <-- many of those functions result in bloaty code IMO.  We should fix the string API to provide things like Find and RFind in a non-obsolete way ;-)

Comment 22

[YAMASHITA Makoto]

darin, could you check this in? thanks.

Comment 23

[Darin Fisher]

Sorry, I meant to mark this fixed after I commited it a while back.