Closed Bug 379165 Opened 13 years ago Closed 13 years ago

[BeOS] BeOS builds broken in js after commit of bug 378261

Categories

(Core :: JavaScript Engine, defect, critical)

Other
BeOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: doug, Assigned: igor)

References

Details

Attachments

(1 file, 3 obsolete files)

Commit of bug 378261 has broken BeOS builds in JS.  Error output is:
In file included from mozilla/js/src/jsapi.h:47,
                 from mozilla/js/src/jsapi.c:55:
mozilla/js/src/jspubtd.h:728: argument format specified for non-function `JSPrintfFormater'
/
Assignee: nobody → general
Component: XPConnect → JavaScript Engine
QA Contact: xpconnect → general
(In reply to comment #0)
> Commit of bug 378261 has broken BeOS builds in JS.  Error output is:
> In file included from mozilla/js/src/jsapi.h:47,
>                  from mozilla/js/src/jsapi.c:55:
> mozilla/js/src/jspubtd.h:728: argument format specified for non-function
> `JSPrintfFormater'

The code in question contains:

typedef int
(* JS_DLL_CALLBACK JSPrintfFormater)(void *closure, const char *format, ...)
#if defined __GNUC__
    __attribute__ ((format (printf, 2, 3)))
#endif
;

It seems that GCC BeOS uses does not support the format attribute for function pointers. So what version of GCC was that?
Assignee: general → igor
We are stuck with gcc 2.95.3 as long as we are supporting BeOS, Haiku will in the future support gcc4.
Blocks: 356310
Attached patch Fix v0.1 (obsolete) — Splinter Review
Does this compiles with BEOS? The patch changes the usage of format attribute to be the right one starting at least with GCC 3.0, see the last 2 paragraphs in http://gcc.gnu.org/onlinedocs/gcc-3.0.4/gcc_5.html#SEC95 , but it is not clear from GCC 2.95 manual if this is supported there.
No. I see the same error as before the patch.
(In reply to comment #4)
> No. I see the same error as before the patch.

OK, it means that the macro has to check that GCC version is at least 3.0.
The policy for regressions is to block, not to depend on the original bug.
Blocks: 378261
No longer depends on: 378261
sorry for mixing up block and depend...
Attached patch Simpler Dump v1 (obsolete) — Splinter Review
Instead of fixing the typedef I suggest to remove it altogether and change JS_DumpHeap to accept the FILE * argument directly. The callback/closure hack came from my tendency to write overgeneric code coupled with an assumption that jsapi.g should not depend on stdio.h. Since in fact jsapi.h includes stdio.h, there is no point not to pass the file argument.
Attachment #263186 - Attachment is obsolete: true
Attachment #263187 - Flags: review?(brendan)
Simpler Dump v1 builds cleanly under BeOS gcc 2.95.3.  Thank you!
I want to note that this can cause problems when mixing CRTs...
Attached patch Simpler Dump v2 (obsolete) — Splinter Review
Here is a version that includes js/src/xpconnect/src/nsXPConnect.cpp that was missing from the previous patch.
Attachment #263187 - Attachment is obsolete: true
Attachment #263191 - Flags: review?(brendan)
Attachment #263187 - Flags: review?(brendan)
Comment on attachment 263191 [details] [diff] [review]
Simpler Dump v2


>@@ -2191,54 +2190,53 @@ DumpNode(JSDumpingTracer *dtrc, JSHeapDu
>         following = node->parent;
>         node->parent = prev;
>         prev = node;
>         node = following;
>         if (!node)
>             break;
>         if (chainLimit == 0) {
>-            if (format(closure, "...") < 0)
>+            if (fprintf(fp, "...") < 0)

fputs here, no formatting required.


> [...]
>-    return ok && format(closure, "\n") >= 0;
>+    return ok && fputc('\n', fp) >= 0;

putc works too, may be a macro where fputs is guaranteed not to be a macro.

> /*
>  * DEBUG-only method to dump an object graph of heap-allocated things.
>  *
>+ * fp: file for the dump output.
>  * start: when non-null, dump only things reachable from start thing. Otherwise
>  *        dump all things rechable from runtime roots.
>  * startKind: trace kind of start if start is not null. Must be 0 when start
>  *            is null.
>  * thingToFind: dump only paths in the object graph leading to thingToFind
>  *              when non-null.
>  * maxDepth: the upper bound on the number of edges to descend from the graph
>  *           roots.
>  * thingToIgnore: thing to ignore during graph traversal when non-null.

Nit: nicer if tabulated so descriptions all start in the same column (1 mod 4 if possible).

r=me with these picked.

/be
Attachment #263191 - Flags: review?(brendan) → review+
Attached patch Simpler Dump v3Splinter Review
Here is patch to commit with nits addressed.
Attachment #263191 - Attachment is obsolete: true
Attachment #263214 - Flags: review+
I committed the patch from comment 13 to the trunk:

Checking in js/jsd/jsd_xpc.cpp;
/cvsroot/mozilla/js/jsd/jsd_xpc.cpp,v  <--  jsd_xpc.cpp
new revision: 1.80; previous revision: 1.79
done
Checking in js/src/js.c;
/cvsroot/mozilla/js/src/js.c,v  <--  js.c
new revision: 3.140; previous revision: 3.139
done
Checking in js/src/jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.317; previous revision: 3.316
done
Checking in js/src/jsapi.h;
/cvsroot/mozilla/js/src/jsapi.h,v  <--  jsapi.h
new revision: 3.147; previous revision: 3.146
done
Checking in js/src/jspubtd.h;
/cvsroot/mozilla/js/src/jspubtd.h,v  <--  jspubtd.h
new revision: 3.81; previous revision: 3.80
done
Checking in js/src/xpconnect/shell/xpcshell.cpp;
/cvsroot/mozilla/js/src/xpconnect/shell/xpcshell.cpp,v  <--  xpcshell.cpp
new revision: 1.96; previous revision: 1.95
done
Checking in js/src/xpconnect/src/nsXPConnect.cpp;
/cvsroot/mozilla/js/src/xpconnect/src/nsXPConnect.cpp,v  <--  nsXPConnect.cpp
new revision: 1.106; previous revision: 1.105
done
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.