Closed
Bug 399647
Opened 17 years ago
Closed 17 years ago
OS/2 build break in nsprpub/pr/src/io/prlog.c
Categories
(NSPR :: NSPR, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wuno, Assigned: mozilla)
References
Details
Attachments
(2 files, 2 obsolete files)
3.23 KB,
patch
|
mozilla
:
review+
|
Details | Diff | Splinter Review |
904 bytes,
patch
|
benjamin
:
review+
benjamin
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (OS/2; U; Warp 4.5; en-US; rv:1.9a9pre) Gecko/2007101000 Minefield/3.0a9pre
Build Identifier:
E:/usr/src/mozilla/nsprpub/pr/src/io/prlog.c: In function `PR_LogPrint':
E:/usr/src/mozilla/nsprpub/pr/src/io/prlog.c:495: error: assignment of read-only location
E:/usr/src/mozilla/nsprpub/pr/src/io/prlog.c:495: error: assignment of read-only location
E:/usr/src/mozilla/nsprpub/pr/src/io/prlog.c:497: warning: implicit declaration
of function `md_UnlockAndPostNotifies'
make.exe[6]: *** [prlog.o] Error 1
Reproducible: Always
Steps to Reproduce:
1.
2.
3.
Reporter | ||
Comment 1•17 years ago
|
||
the offending line
http://mxr.mozilla.org/seamonkey/source/nsprpub/pr/src/io/prlog.c#495
Depends on: 244478
Reporter | ||
Comment 2•17 years ago
|
||
I have no clue if that's allowed, but it works with regard to compilation and a working browser
Comment 3•17 years ago
|
||
Confirming bug on WinXP with cygwin.
Also agree that this bug was introduced by fix for Bug #244478
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•17 years ago
|
||
Hi. Please try this:
494 if (!nb || (line_long[nb-1] != '\n')) {
char eol[2];
eol[0] = '\n';
eol[1] = '\0';
495 _PUT_LOG(logFile, eol, 1);
496 }
Thanks!
Comment 5•17 years ago
|
||
The attached patch doesn't work for me. Different OS, etc, probably the cause.
However, the patch in Comment #4 allows code to compile.
A complete build is several hours away....
Assignee | ||
Comment 6•17 years ago
|
||
Yes, with the eol variable it works on OS/2. The shorter
if (!nb || (line_long[nb-1] != '\n')) {
char eol[] = "\n\0";
_PUT_LOG(logFile, eol, 1);
}
also works.
I still get a warning about the following line _PR_UNLOCK_LOG();
M:/trunk/mozilla/nsprpub/pr/src/io/prlog.c:498: warning:
implicit declaration of function `md_UnlockAndPostNotifies'
but that doesn't seem to be harmful -- and not easily fixed without starting to include .c files...
Reporter | ||
Comment 7•17 years ago
|
||
Comment on attachment 284691 [details] [diff] [review]
change of 1 and "\n" compiles
confirming also that the Wan-Teh's patch works. It took me a while to come back, since the browser crashes at certain pages but that seems not to be related to this bug (an older non-crashing build runs stable with the nspr dll's compiled using this patch)
Attachment #284691 -
Attachment is obsolete: true
Comment 8•17 years ago
|
||
Peter, the initialization of an automatic (as opposed to static)
array is a new feature of C. This is why I use assignments.
You can fix the undeclared function warning in two ways.
1. In _os2.h, declare md_UnlockAndPostNotifies before the
definition of _MD_UNLOCK:
/* --- Lock stuff --- */
#define _PR_LOCK _MD_LOCK
#define _PR_UNLOCK _MD_UNLOCK
+ extern void md_UnlockAndPostNotifies(_MDLock *lock, PRThread *waitThred,
+ _MDCVar *waitCV);
#ifdef USE_RAMSEM
#define _MD_NEW_LOCK (_PR_MD_NEW_LOCK)
2. Define _MD_UNLOCK as a function:
In _os2.h, do this:
#define _MD_UNLOCK (_PR_MD_UNLOCK)
Define the _PR_MD_UNLOCK function in os2cv.c.
Assignee | ||
Comment 9•17 years ago
|
||
OK, this now compiles without warnings but I haven't really tested it yet. (The USE_RAMSEM part is not interesting any more, I should actually update the patch in bug 330720 to remove it completely.)
Comment 10•17 years ago
|
||
Comment on attachment 284786 [details] [diff] [review]
fix build and remove warning
r=wtc.
>+void _PR_MD_UNLOCK(_MDLock *lock)
>+{
>+ if (0 != (lock)->notified.length) {
>+ md_UnlockAndPostNotifies((lock), NULL, NULL);
>+ } else {
>+ DosReleaseMutexSem((lock)->mutex);
>+ }
>+}
When defined as a function, you can remove the parentheses
around the parameter 'lock'.
Attachment #284786 -
Flags: review+
Assignee | ||
Comment 11•17 years ago
|
||
Of course, this removes the brackets. Carrying over r+. (In the meantime I also tested PR logging, to confirm that after the build break fix it works again.)
Do I still need a sr for NSPR HEAD?
Assignee: wtc → mozilla
Attachment #284786 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #284995 -
Flags: review+
Comment 12•17 years ago
|
||
Comment on attachment 284995 [details] [diff] [review]
[checked in] fix v2
Peter, please go ahead and check in this patch on the NSPR trunk.
Then, please create a new static tag NSPR_HEAD_yyyymmdd for mozilla/client.mk
and request approval. (Mozilla trunk is pulling the NSPR tag specified in
mozilla/client.mk.)
Assignee | ||
Comment 13•17 years ago
|
||
Comment on attachment 284995 [details] [diff] [review]
[checked in] fix v2
Checked this into NSPR trunk.
Attachment #284995 -
Attachment description: fix v2 → [checked in] fix v2
Assignee | ||
Comment 14•17 years ago
|
||
OK, to activate this fix on trunk, we need to move the NSPR tag. The only change on NSPR trunk between these tags is the OS/2 change of attachment 284995 [details] [diff] [review].
Attachment #285121 -
Flags: review?(benjamin)
Attachment #285121 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #285121 -
Flags: review?(benjamin)
Attachment #285121 -
Flags: review+
Attachment #285121 -
Flags: approval1.9?
Attachment #285121 -
Flags: approval1.9+
Assignee | ||
Comment 15•17 years ago
|
||
Comment on attachment 285121 [details] [diff] [review]
[checked in] Move NSPR tag in client.mk
Checked this into trunk.
Attachment #285121 -
Attachment description: Move NSPR tag in client.mk → [checked in] Move NSPR tag in client.mk
Assignee | ||
Comment 16•17 years ago
|
||
With that this bug should be fixed.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•