84584 - Dependency on XP_WIN in files that don't include secport.h

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

Mike Belshe wrote:

> I recently ran into some similar problems with several types of ptrs in
> functionlists being bogus...
> 
> I traced it down to this little nasty in pkcs11p.h & pkcs11u.h:
> 
>     #if defined(XP_WIN)
>     #if defined(_WIN32)
>     #pragma warning(disable:4103)
>     #pragma pack(push, cryptoki, 1)
>     #endif
>     #endif
> 
> The makesystem I was using for my stuff wasn't defining "XP_WIN", so I
> didn't get the packing right and all the offsets were messed up relative to
> nss3.dll, which did define XP_WIN...
> 
> Mozilla guys- this appears to be the only reference to XP_WIN in the public
> headers for PKCS11.... any chance it can be redone as just _WIN32 but not
> XP_WIN?  That might cause other problems... who knows :-)
> 
> Mike

Coreconf doesn't define XP_WIN on the cc command line.  
XP_WIN is defined in secport.h.  But the pkcs11 header files mentioned above
don't include secport.h.  So, if they get #included by some source file,
they only work correctly if that source has previously #included secport.h.
pkcs11[pu].h only get included by pkcs11.h and pkcs11t.h, neither of which
include secport.h first (AFAIK)!

I think the reference to XP_WIN there is completely redundant and should 
simply be removed.  Agreed?

Comment 1

[Wan-Teh Chang]

XP_WIN means Win32 or Win16, so the reference to XP_WIN
there is redundant; _WIN32 (defined by the compiler) is
enough.

Nelson, can you fix this?

On the other hand, shouldn't every NSS header include
secport.h?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

I wouldn't say that every NSS header should include it, but I'd say that
those NSS headers that depend on the symbols it defines should include it.

I'm reassigning to myself.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Fixed by checkins on trunk to pkcs11[pu].h 
Other occurrances of XP_WIN are limited to private headers.