Closed Bug 53902 Opened 19 years ago Closed 19 years ago

Mozilla M17 does not build on NetBSD/macppc

Categories

(NSPR :: NSPR, defect, P3)

PowerPC
NetBSD
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: martin, Assigned: wtc)

Details

Attachments

(1 file)

Someone seems to have copied some linux/ppc specific stuff over and assumed,
NetBSD would be the same. This is not the case: a va_list is not an array on
NetBSD/macppc.

Applying the below patch fixes this.

*** mozilla/nsprpub/pr/src/io/prprf.c.orig      Fri Jul 16 02:30:32 1999
--- mozilla/nsprpub/pr/src/io/prprf.c   Sat Sep 23 10:59:22 2000
***************
*** 34,44 ****
  /*
  ** Note: on some platforms va_list is defined as an array,
  ** and requires array notation.
  */
  #if (defined(LINUX) && defined(__powerpc__)) || defined(WIN16) || \
!     defined(QNX) || (defined(__NetBSD__) && defined(__powerpc__))
  #define VARARGS_ASSIGN(foo, bar) foo[0] = bar[0]
  #else
  #define VARARGS_ASSIGN(foo, bar) (foo) = (bar)
  #endif

--- 34,44 ----
  /*
  ** Note: on some platforms va_list is defined as an array,
  ** and requires array notation.
  */
  #if (defined(LINUX) && defined(__powerpc__)) || defined(WIN16) || \
!     defined(QNX)
  #define VARARGS_ASSIGN(foo, bar) foo[0] = bar[0]
  #else
  #define VARARGS_ASSIGN(foo, bar) (foo) = (bar)
  #endif
Thanks for the bug report.

The cvs checkin comments for prprf.c show the following:
-=-=-=-=-=-=-=-=-=-=-=-=
revision 3.7
date: 1999/07/16 00:30:32;  author: srinivas%netscape.com;  state: Exp;  lines:
+2 -1
NetBSD/PowerPC port. Checkin for kei_sun@ba2.so-net.ne.jp
-=-=-=-=-=-=-=-=-=-=-=-=

Could this be a difference between the macppc and ofppc
ports of NetBSD?  Or maybe this depends on the NetBSD
version number?
This seems to have been "fixed" in the NetBSD powerpc ports earlier this year to
comply with the PowerPC ABI spec. All ports of NetBSD with a PowerPC CPU use the
same (cpu-dependend) va_list implementation.
But may I suggest a different solution: there is a va_copy() macro in C99 (and
already implemented in gcc for quite some time). It seems to be doing exactly
what you are trying to do here. This would be cleaner and avoid a NetBSD
specific version check. But if you prefer to do the version check, I can dig for
the right ifdef...
The va_copy() solution sounds interesting, but
now I'm inclined to just checking in your patch,
assuming that the problem that the old patch
was trying to solve has been fixed in the NetBSD
powerpc ports earlier this year.

If you can dig up the version in which this was
fixed, that will be great.  But don't worry about
it if that's too much trouble.

In the long term, I want to rewrite the relevant
code to completely avoid copying a va_list.
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OK, the change seemed to happen at 1.4T, but since noone runs 1.4-current now
any more, I'd simplify on 1.4 = old/broken, 1.5 and newer = OK. The defines come
from sys/param.h, which are already included here (I think). The check would be:

#if defined(__NetBSD_Version__) && __NetBSD_Version__ >= 105000000
 // new code, no [0] needed
#else
...
#endif
> OK, the change seemed to happen at 1.4T,
> but since noone runs 1.4-current now any
> more, I'd simplify on 1.4 = old/broken,
> 1.5 and newer = OK.

Does this depend on the processor type?
In other words, do I need to add defined(__powerpc__)
to your check?
Sorry, should have been clearer on this: yes, only if __powerpc__ is defined
there is a difference at all. The "generic" version should work for all other
ports of NetBSD, only powerpc has been "broken" and therefor needed special
treatment. In the newer version (v1.5 and above) this is fixed, so no special
handling for any CPU is necessary anymore.
Attached patch Proposed patch.Splinter Review
Thanks for the info.  Please review and try
the attached patch (id=15945).  Note that
my patch takes advantage of the fact that
if a macro is not defined, it has the value
of 0L in a conditional expression.  I also
added #include <sys/param.h> becausw we
were not including that header file before.
The proposed patch is fine.
Thanks for the code review.  Now I need to
figure out how to get the approval to check
this into the appropriate branches. This may
take a few days.
I checked in the patch on the NSPR main trunk,
NSPRPUB_CLIENT_BRANCH, and NSPRPUB_RELEASE_4_0_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.0.2
Version: 3.0 → 4.0
I'm confused - this fix still isn't present in the Mozilla-0.6 snapshot, while
it is OK in the cvs'd version ever since this bug has been marked FIXED.
Different CVS branch?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Netscape 6 was produced on its own branch.
I understand that Mozilla 0.6 was based on
Netscape 6 source code.  This would explain
why the fix is not in Mozilla 0.6.
Bug reporter is happy with my explanation.
Marked the bug fixed again.
Status: REOPENED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.