233493 - symkeyutil build broken on OS/2

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Julien Pierre]

E:\dev\nss\tip\mozilla\security\nss\cmd\symkeyutil\OS22.45_icc_OPT.OBJ\symkeyuti
l.obj(E:/dev/nss/tip/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c) : error L
NK2029: "read" : unresolved external
E:\dev\nss\tip\mozilla\security\nss\cmd\symkeyutil\OS22.45_icc_OPT.OBJ\symkeyuti
l.obj(E:/dev/nss/tip/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c) : error L
NK2029: "open" : unresolved external
E:\dev\nss\tip\mozilla\security\nss\cmd\symkeyutil\OS22.45_icc_OPT.OBJ\symkeyuti
l.obj(E:/dev/nss/tip/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c) : error L
NK2029: "close" : unresolved external
E:\dev\nss\tip\mozilla\security\nss\cmd\symkeyutil\OS22.45_icc_OPT.OBJ\symkeyuti
l.obj(E:/dev/nss/tip/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c) : error L
NK2029: "write" : unresolved external


There were 4 errors detected
gmake.exe: *** [OS22.45_icc_OPT.OBJ/symkeyutil.exe] Error 12


The fix is to use NSPR . Patch forthcoming.

Comment 1

[Julien Pierre]

Attachment: Use NSPR instead of libc functions

This did not break mozilla because it does not build nss/cmd .

Comment 2

[Julien Pierre]

This problem was introduced in 3.9, when the symkeyutil tool was added.
I would like to check this into both NSS_3_9_BRANCH and the tip.

Comment 3

[Robert Relyea]

Comment on attachment 140934 [details] [diff] [review]
Use NSPR instead of libc functions

The patch is basically good, but if we are replacing the libc version of
open/close/read/write we should also replace the perror calls so correct
failures are printed.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Julien, 

there was a similar problem on windows, with these symbols being underfined.  
I fixed them with a small change that undefined __STDC__ on that platform only.
That change causes all those symbols to become defined.
I wonder if there's a similarly simple patch for OS/2.

Comment 5

[Julien Pierre]

Attachment: use PR functions for errors instead of perror

Nelson, I know there is indeed a simpler way to get these libc functions to
work under OS/2. I only reported the link error, but there were also compile
warnings because the header defining them was not included on OS/2.
But there are platforms that don't necessarily have all the needed libc
functions (eg. embedded), so I think the proper fix is to use NSPR as I did in
this patch.

Comment 6

[Robert Relyea]

Comment on attachment 141071 [details] [diff] [review]
use PR functions for errors instead of perror

r=relyea

Comment 7

[Julien Pierre]

Thanks, Bob.

Fixed in tip :

Checking in symkeyutil.c;
/cvsroot/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c,v  <--  symkeyutil.c
new revision: 1.5; previous revision: 1.4

and NSS_3_9_BRANCH :

Checking in symkeyutil.c;
/cvsroot/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c,v  <--  symkeyutil.c
new revision: 1.4.4.1; previous revision: 1.4

Comment 8

[Wan-Teh Chang]

Comment on attachment 141071 [details] [diff] [review]
use PR functions for errors instead of perror

I suggest the following changes.

1. Change "io.h" to <io.h> because it is a standard
system header file.  Actually, I think the <io.h>
inclusion (and possibly <unistd.h>, <sys/types.h>,
<sys/stat.h>, and <fcntl.h>) and the O_BINARY macro
definition can be deleted altogether.

2. If we include "nspr.h", we don't need to include
"prtypes.h", "prtime.h", and "prlong.h".

3. Add 'static' to all the functions that should be 'static'.

4. The new code should follow the prevalent style for the
placement of curly braces in the file.

5. Use SECU_PrintError (declared in "secutil.h") instead
of 'PrintError'.  Delete 'PrintError'.

Comment 9

[Julien Pierre]

Attachment: incremental patch

This fixes all of Wan-Teh's issues except for the "static" keyword. symkeyutil
is an executable consisting of a single C source file, so the static keyword
would not have much effect.

Comment 10

[Julien Pierre]

Checked in the incremental patch to tip

Checking in symkeyutil.c;
/cvsroot/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c,v  <--  symkeyutil.c
new revision: 1.6; previous revision: 1.5

and branch

Checking in symkeyutil.c;
/cvsroot/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c,v  <--  symkeyutil.c
new revision: 1.4.4.2; previous revision: 1.4.4.1

Comment 11

[Wan-Teh Chang]

Comment on attachment 141126 [details] [diff] [review]
incremental patch

The first argument to SECU_PrintError should be the
name of the program (symkeyutil).

Comment 12

[Julien Pierre]

I know, but the program name variable wasn't shared anywhere so I got lazy and
used the precedent that many other nss/cmd programs have by setting the first
argument to "Error".

Comment 13

[Julien Pierre]

Checking in symkeyutil.c;
/cvsroot/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c,v  <--  symkeyutil.c
new revision: 1.7; previous revision: 1.6

Checking in symkeyutil.c;
/cvsroot/mozilla/security/nss/cmd/symkeyutil/symkeyutil.c,v  <--  symkeyutil.c
new revision: 1.4.4.3; previous revision: 1.4.4.2