23763 - jsotypes.h contains bogus arguments

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect, P3)

Description

[Jim Salter]

In the jsotypes.h, it uses coding structure similar to NSPR's
obsolete/protypes.h.  However, this structure doesn't work for AIX 4.3.x which
provides the int types of int32, int64, etc.  The latest version of NSPR's
obsolete/protypes.h includes the following code:

  /*
   * On AIX 4.3, sys/inttypes.h (which is included by sys/types.h, a very
   * common header file) defines the types int8, int16, int32, and int64.
   * So we don't define these four types here to avoid conflicts in case
   * the code also includes sys/types.h.
   */
   #if defined(_PR_AIX_HAVE_BSD_INT_TYPES)
   /* <sys/types.h> is already included */
   #else

Something similar to this should be done for jsotypes.h.

Comment 1

[Jim Salter]

See bug 23304 for more information.

Comment 2

[Norris Boyd]

Rhino isn't the C JavaScript engine. Reassigning to the C engine component.

Comment 3

[Mike McCabe]

Passing on to Rogerl.

Comment 4

[leger]

Setting all Javacript bugs to rginda QA Contact.

Comment 5

[Jim Salter]

This needs to be fixed if the use of NSPR continues.

Comment 6

[Wan-Teh Chang]

This Javascript header (jsotypes.h) is independent of
NSPR.

In jsotypes.h, you need to change the macro AIX4_3
to a macro (e.g., JS_AIX_HAVE_BSD_INT_TYPES) that is
defined in the jscpucfg.h file for AIX 4.3.  Then,
modify jscpucfg.c to generate the line
    #define JS_AIX_HAVE_BSD_INT_TYPES 1
on AIX 4.3.

Comment 7

[Phil Schwartau]

Taking over QA for the JavaScript Engine -

I have raised this issue with the team. If either Jim or Wan-Teh have the 
correct solution for this, the JS Engine team will review it and incorporate it.
In such a case, please attach the patch to this bug, and it will be taken up.  

Apologies for the delay on this!

Comment 8

[Jim Salter]

I think Wan-Teh has accurately described the fix.  I do not have permissions to 
check into the tree, so if Wan-Teh will make the change, I can review it and 
verify it.

Comment 9

[Wan-Teh Chang]

I described my fix in my comments dated 2000-05-03 15:58.
I don't have it in the form of a patch file.

Comment 10

[rogerl (gone)]

The change in jsotypes.h should be from AIX4_3 to JS_AIX_HAVE_BSD_INT_TYPES, but 
then that define needs to occur in config/AIX4.3.mk. (jscpucfg.h is not valid on  
unix builds). That means 4.3.X users will need to modify the makefile 
appropriately, or make a new one.

Comment 11

[Patrick C. Beard]

Couldn't this go on the JS_150_RC2 branch? It isn't crucial for beta3, is it?

Comment 12

[Jim Salter]

Attachment: config and source diffs

Comment 13

[Jim Salter]

Come on - this one should be easy to resolve.  Can someone please "own" getting 
this checked in?  Since I don't have permissions to check in the fix, I'd like 
some help, here.

Comment 14

[rogerl (gone)]

I'll ride the checkin if we can get some r= from anybody?
Phil, can you apply the patch on an AIX box and try it out for sanity's sake?

Comment 15

[Brendan Eich [:brendan]]

ob. dumb question: why is the AIX build on
http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey-Ports green?

The patch (diff -u format please) should not be defining a JS_ macro in an
nsprpub/.../*.mk file.  Shouldn't it be updating js/src/config/AIX4.3.mk?

/be

Comment 16

[Brendan Eich [:brendan]]

Answering my own second (dumb) question: js/src/config/AIX*.mk are used only by
the standalone JS build driven from js/src/Makefile.ref.  We do need to keep the
standalone build working, but it should get fixed for free if we follow wtc's
advice.

So Jim's patch does not implement wtc's suggestion -- in particular, there are
no jscpucfg.c changes, and an NSPR .mk file instead defines a JS_FOO macro.  The
right patch is easy enough, given wtc's description and the NSPR precedent on
which all this jsotypes.h junk is based.  Will someone please hack the right
patch and attach it?

/be

Comment 17

[Jim Salter]

Please see wan-teh's comment earlier in this bug.  I did not find any file 
named config/AIX4.3.mk (though there is a security/coreconf/AIX4.3.mk, but 
that's not what I thought we needed to change), so I didn't change that.

However, within the nsprpub/config/AIX.mk file, there's an OS_RELEASE 
comparison with 4.3, so that's where I put the code.

Comment 18

[Jim Salter]

oh, I see what was required - yes, please ignore my attachment - it's wrong for 
this fix.

And what do you mean by the tinderbox being green?  It looks yellow to me...

Comment 19

[Brendan Eich [:brendan]]

The config/AIX4.3.mk file I mentioned is under js/src.  It's used for the
standalone JS build only (js/src/Makefile.ref).

I did read (and re-read) wtc's comments, and still see his only specific advice
as modifying js/src/jscpucfg.c to generate the right macro (if self-hosting on
AIX4.3) in jscpucfg.h.  wtc's very first sentence is still relevant: all this
mess is independent of NSPR.

Therefore and again, I don't think NSPR .mk files should define JS_* macros in
any event.

/be

Comment 20

[Brendan Eich [:brendan]]

Sorry, I didn't see a mid-air collision, must have failed to read Jim's latest
comments.

Tinderbox (host "hotaix") looks green to me, new cycle in progress.

/be

Comment 21

[Brendan Eich [:brendan]]

Can we get rid of the nsbeta3 noise in Keywords and Status Whiteboard?

/be

Comment 22

[Phil Schwartau]

I've deleted the "nsbeta3" 's  from Keywords and Status Whiteboard - 

Comment 23

[Patrick C. Beard]

Marking future.

Comment 24

[Philip K. Warren]

Attachment: Potential fix

Is this the type of fix you are looking for on this bug? Instead of adding the
JS_AIX_HAVE_BSD_TYPES declaration to jscpucfg.c, I created it in jsosdep.h
(suggested by Mike Kaply).

Comment 25

[Wan-Teh Chang]

Philip,

JS_AIX_HAVE_BSD_TYPES needs to be defined in a public
header file.  I believe jscpucfg.h is public and jsosdep.h
is private.  This is why I suggested defining
JS_AIX_HAVE_BSD_TYPES in jscpucfg.h in comment #6.

Comment 26

[Mike Kaply [:mkaply]]

jscpucfg.h doesn't seem to have any compiler/machine stuff in it.

It would seem to me that jsosdep.h is exactly the file for this.

It says at the top:

 37 /*
 38  * OS (and machine, and compiler XXX) dependent information.
 39  */

And jscpucfg.h includes jsosdep.h right at the top.

Comment 27

[Wan-Teh Chang]

If jscpucfg.h includes jsosdep.h, then jsosdep.h is
public.  Then your patch is fine.  Sorry about my
confusion.

Comment 28

[Brendan Eich [:brendan]]

Comment on attachment 91113 [details] [diff] [review]
Potential fix

sr=[e-mail] if this works as desired on all AIX systems.  I had
thought there was a need to define JS_AIX_HAVE_BSD_INT_TYPES based on narrower
conditions than just AIX4_3 being defined (like, only for 4.3.x for x >= some
number), but if this is good enough, let's get it in for js1.5.

/be

Comment 29

[Bob Clary [:bc] (inactive)]

mkaply, can you find someone to take this bug and do the required testing?

Comment 30

[Jim Salter]

I don't have any AIX system to test against, and this has been open for 8 years without progress, so i'm going to close this old bug.