Status

NSPR
NSPR
--
enhancement
RESOLVED WONTFIX
15 years ago
6 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

15 years ago
This has been driving me crazy for a while

PR_IMPLEMENT(void) PR_Assert(const char *s, const char *file, PRIntn ln)
{
all:       PR_LogPrint("Assertion failure: %s, at %s:%d\n", s, file, ln);
unix|os/2: fprintf(stderr, "Assertion failure: %s, at %s:%d\n", s, file, ln);
mac:       dprintf("Assertion failure: %s, at %s:%d\n", s, file, ln);
win|os/2:  DebugBreak();
!mac:      abort();
}

DebugBreak on windows gives an abort button.
DebugBreak on os/2 emx is implemented as Crash.
DebugBreak on os/2 va is presumably implemented in some sane manner.

Of note: Debugger() is available on MacOS(X). dprintf isn't preferred on OSX.
I'll attach a proposed patch.

Yes, that's right, on MacOS classic PR_Assert is never fatal and doesn't even
dump you into your debugger.
(Assignee)

Comment 1

15 years ago
Created attachment 108511 [details] [diff] [review]
draft

Comment 2

15 years ago
Created attachment 108628 [details]
build log for prlog.o

Comment 3

15 years ago
ignore previously attached buildlog, will redo

Comment 4

15 years ago
Created attachment 108629 [details]
build log for prlog.o
Attachment #108628 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #108511 - Flags: review?(wtc)

Comment 5

15 years ago
Comment on attachment 108511 [details] [diff] [review]
draft

1. The abort() call must not be removed.  A PR_ASSERT
failure is fatal.  We provide the hooks to the debugger
merely as a convenience.

2. The XP_MACOSX change is not necessary.  In this case
Mac OS X is treated as a Unix variant.

3. The OutputDebugString() calls for WIN32 won't be
necessary if we are also calling fprintf() for WIN32.
I think we should just add || defined(WIN32) to the
long ifdef before the fprintf() statement.

4. Is the dprintf() call for the Mac printing the
assertion failure message anywhere?  If not, maybe
we should just delete it?
Attachment #108511 - Flags: review?(wtc) → review-
(Assignee)

Comment 6

15 years ago
> The abort() call must not be removed. 
> A PR_ASSERT failure is fatal. 
except on macos where it's ignored.
> We provide the hooks to the debugger merely as a convenience.
1. i'll put the abort back, but i want to put it back w/o the !MAC, ok?

> The XP_MACOSX change is not necessary.
2. Ok.

> The OutputDebugString() calls for WIN32 won't be
> necessary if we are also calling fprintf() for WIN32.
> I think we should just add || defined(WIN32) to the
> long ifdef before the fprintf() statement.
3. In release builds there's no console. Where would fprintf(stderr) go?
Using outputdebugstring allows you to catch error output w/ 3rd party utilities
(sysinternals and microsoft have some iirc).

> Is the dprintf() call for the Mac printing the
> assertion failure message anywhere?  If not, maybe
> we should just delete it?
4. Sorry, my 66mhz powermac which actually did build nspr died a while back, i
need to buy a new hard drive (to replace the 1/2gb scsi disk) and a boxset of
macos9 (unless someone can get me those things).

I think dprintf goes to macsbug/codewarrior/mpw and other things, so I'm not
going to remove it unless someone indicates that it went nowhere.
--
At this point i don't see a need to attach a new patch since the only changes
are putting back the |abort();| line and not adding XP_MACOSX.
Status: NEW → ASSIGNED
(Assignee)

Comment 7

15 years ago
Created attachment 116443 [details] [diff] [review]
PR_Assert will now be fatal on MacOS
Attachment #108511 - Attachment is obsolete: true
Attachment #108629 - Attachment is obsolete: true
(Assignee)

Updated

15 years ago
Attachment #116443 - Flags: review?(wtc)
QA Contact: wtchang → nspr

Comment 8

10 years ago
timeless, what is the state of this bug, its patch and the review request? Still the same?
(Assignee)

Updated

7 years ago
Duplicate of this bug: 612093
(Assignee)

Comment 10

7 years ago
Simon: dunno, please bug wtc, there's a review request and he hasn't responded. That's not atypical, but I am not willing to be seen as the bad guy constantly nagging people for reviews. It's unfair to ask me to do this. We have a nice request queue system which makes it easy to see who is sitting on requests.
Isn't Mac Classic dead?

Comment 12

7 years ago
Marked the bug WONTFIX.  NSPR no longer supports
Mac Classic, and I'll deal with Windows in bug 612093.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WONTFIX

Updated

6 years ago
Attachment #116443 - Flags: review?(wtc)
You need to log in before you can comment on or make changes to this bug.