455348 - Change hyphens to underscores in DEBUG_$(shell whoami).

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[Wan-Teh Chang]

I suggest that the coreconf build system not define DEBUG_$(shell whoami)
(or its equivalent for platforms that don't have the 'whoami' command),
and we remove DEBUG_userid from the NSS source tree.

The reason is that we need to process the user id if it contains
characters that are illegal in a macro name, such as spaces and
hyphens.  On my home computer, my user id is "Wan-Teh Chang", so
I need to substitute the '-' and ' ' by an underscore '_', or
patch coreconf to not define the macro.

I never use DEBUG_wtc during development, and don't think
such conditionally compiled code should be checked in.

The NSS source tree contains these DEBUG_userid macros:
DEBUG_kaie
DEBUG_nb95248
DEBUG_jp96085
DEBUG_nelsonb
DEBUG_volvov
DEBUG_volkov1
DEBUG_stevep
DEBUG_jpierre

The PSM source tree contains these DEBUG_userid macros:
DEBUG_javi
DEBUG_jgmyers

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, 
My userid is not nelsonb on most of the computers on which I build, but
I have a script that sets a number of environment variables that I commonly
use when I build, and one of the variables it sets is USERNAME=nelsonb

This feature has a long history, and as you can see, many people have used it.
We can see who will be inconvenienced by removing it.
I wonder: who (or what) benefits from removing it?

Comment 2

[Wan-Teh Chang]

I benefit from removing it.  If any of you really needs it, I'll just
submit a patch to substitute hyphens by underscores.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Speaking for myself only, #ifdef DEBUG_nelsonb appears in only 3 places in NSS.
Two of them can be deleted.  One can be converted into simple ifdef DEBUG, I
think.

In sslsnce.c, these lines can be deleted:

1182 #if defined(DEBUG_nelsonb)
1183     printf("sizeof(sidCacheEntry) == %u\n", sizeof(sidCacheEntry));
1184 #endif

In selfserv.c, these lines can be deleted:

2064 #ifdef DEBUG_nelsonb
2065         WaitForDebugger();
2066 #endif

and the following #ifdef can be converted to #ifdef DEBUG

1769 #ifdef DEBUG_nelsonb
1770 
1771 #if defined(XP_UNIX) || defined(XP_OS2) || defined(XP_BEOS)
1772 #define SSL_GETPID getpid
1773 #elif defined(_WIN32_WCE)
1774 #define SSL_GETPID GetCurrentProcessId
1775 #elif defined(WIN32)
1776 extern int __cdecl _getpid(void);
1777 #define SSL_GETPID _getpid
1778 #else
1779 #define SSL_GETPID() 0   
1780 #endif
...

and the function WaitForDebugger can be deleted.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Attachment: remove DEBUG_nelsonb from NSS sources (checked in)

There was a time when I valued these things, but not any more.  
So, I'm ridding NSS of them.  Wan-Teh, please review.

Comment 5

[Wan-Teh Chang]

Comment on attachment 340279 [details] [diff] [review]
remove DEBUG_nelsonb from NSS sources (checked in)

r=wtc.

Comment 6

[Julien Pierre]

Attachment: change to my current userid

I would like to keep this, as well as the assertion code in secport.c that's in an ifdef block under my name.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Wan-Teh, does changing DEBUG_$(shell whoami) to DEBUG_${USERNAME} in that
makefile help you?

Comment 8

[Wan-Teh Chang]

Attachment: Proposed patch: change hyphens to underscores

The solution is to change characters that are illegal in a macro name
to underscores.  On Unix, usernames can't contain spaces, so we only
need to change hyphens.

Comment 9

[Wan-Teh Chang]

Comment on attachment 340483 [details] [diff] [review]
Proposed patch: change hyphens to underscores

Nelson, switching to ${USERNAME} won't help.

I didn't realize USERNAME is an environment variable on Unix.
Should the variable in my patch be renamed USERID to avoid
confusion?

Comment 10

[Julien Pierre]

Wan-Teh,

USERNAME is not an environment variable on my Solaris box. USER is . USERID is not . I don't know if it's standardized .

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 340476 [details] [diff] [review]
change to my current userid

r=nelson

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 340279 [details] [diff] [review]
remove DEBUG_nelsonb from NSS sources (checked in)

Checking in cmd/selfserv/selfserv.c; new: 1.88; previous: 1.87
Checking in lib/ssl/sslsnce.c;       new: 1.46; previous: 1.45

Comment 13

[Julien Pierre]

Thanks for the review, Nelson.

Checking in nss/cmd/crlutil/crlutil.c;
/cvsroot/mozilla/security/nss/cmd/crlutil/crlutil.c,v  <--  crlutil.c
new revision: 1.31; previous revision: 1.30
done

Can we mark this bug fixed ?

Comment 14

[Wan-Teh Chang]

I checked in my patch (attachment 340483 [details] [diff] [review]) on the NSS trunk (NSS 3.12.2).

Checking in UNIX.mk;
/cvsroot/mozilla/security/coreconf/UNIX.mk,v  <--  UNIX.mk
new revision: 1.6; previous revision: 1.5
done

I changed the summary of the bug report to reflect what we decided to do.