187180 - VARARGS_ASSIGN should use va_copy() if available

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect, P3)

Description

[Iain MacDonnell]

User-Agent:       Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.3b) Gecko/20021228
Build Identifier: Mozilla/5.0 (X11; U; SunOS sun4u; en-US; rv:1.3b) Gecko/20021228


These three source files:

M mozilla/js/src/jsprf.c
M mozilla/js/src/xpconnect/src/xpcconvert.cpp
M mozilla/xpcom/ds/nsTextFormatter.cpp

define the macro VARARGS_ASSIGN as:

#ifdef HAVE_VA_LIST_AS_ARRAY
#define VARARGS_ASSIGN(foo, bar)        foo[0] = bar[0]
#else
#define VARARGS_ASSIGN(foo, bar)        (foo) = (bar)
#endif

but this is not fully portable - building with forte 6.2 on Solaris, this
gives error:

"jsprf.c", line 615: warning: pointer to void or function used in arithmetic
"jsprf.c", line 615: warning: pointer to void or function used in arithmetic
"jsprf.c", line 615: operand cannot have void type: op "="

Why isn't va_copy() used if it is available? Changing the macro def to:

#define VARARGS_ASSIGN(foo, bar)        va_copy(foo, bar)

allows the compile to proceed.


Reproducible: Always

Steps to Reproduce:
1.build using forte tools on Solaris
2.
3.

Actual Results:  
"jsprf.c", line 615: warning: pointer to void or function used in arithmetic
"jsprf.c", line 615: warning: pointer to void or function used in arithmetic
"jsprf.c", line 615: operand cannot have void type: op "="

...etc..

Expected Results:  
erm, built properly!

I suggest that the configure script does a check to see if va_copy is available,
and defines HAVE_VA_COPY if so, then change the macro def to:

#ifdef HAVE_VA_COPY
#define VARARGS_ASSIGN(foo, bar)        va_copy(foo, bar)
#elif HAVE_VA_LIST_AS_ARRAY
#define VARARGS_ASSIGN(foo, bar)        foo[0] = bar[0]
#else
#define VARARGS_ASSIGN(foo, bar)        (foo) = (bar)
#endif

or something...




cc: Sun WorkShop 6 update 2 C 5.3 2001/05/15

Comment 1

[hacker formerly known as [e-mail]]

I'm not sure about the portability issue but that construct works fine on our
Forte 6U2 tinderbox, nebiros. See
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Ports/1041040140.3505.gz&fulltext=1

cc: Sun Workshop 6 update 2 C 5.3 Patch 111679-05 2002/02/07

I would suggest applying the neccessary Forte patches.

Comment 2

[Roland Mainz]

1. "cc: Sun WorkShop 6 update 2 C 5.3 2001/05/15"
                                      ^^^^^^^^^^
is really old.
See http://access1.sun.com/sundev/fdp6u2-patches.html for matching patches.

2. Using |va_copy()| may be a good idea (being portable is always good :) ...
brendan ?

Comment 3

[Iain MacDonnell]

OK, I upgraded to:

cc: Forte Developer 7 C 5.4 2002/03/09

and the problem has gone away. I still think va_copy() is a good idea, though...

Comment 4

[hacker formerly known as [e-mail]]

Hrm.  Too bad we can't use the glib macro for this.  I'd hate to have to
copy-n-paste configure checks.

Comment 5

[hacker formerly known as [e-mail]]

Attachment: Add va_copy checks

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 111103 [details] [diff] [review]
Add va_copy checks

Looks ok to me -- I guess the standalone JS build goo will need to be improved
for certain compilers, but I don't know who will do that work.

/be

Comment 7

[Christopher Blizzard (:blizzard)]

Comment on attachment 111103 [details] [diff] [review]
Add va_copy checks

r=blizzard

Comment 8

[hacker formerly known as [e-mail]]

Patch has been checked in.