The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in 3.12.2

Status

NSS
Build
--
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

unspecified
3.12.2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Assignee)

Description

9 years ago
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
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?
(Assignee)

Comment 2

9 years ago
I benefit from removing it.  If any of you really needs it, I'll just
submit a patch to substitute hyphens by underscores.
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.
Created attachment 340279 [details] [diff] [review]
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.
Attachment #340279 - Flags: review?(wtc)
(Assignee)

Updated

9 years ago
Attachment #340279 - Flags: review?(wtc) → review+
(Assignee)

Comment 5

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

r=wtc.

Comment 6

9 years ago
Created attachment 340476 [details] [diff] [review]
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.
Attachment #340476 - Flags: review?(nelson)
Wan-Teh, does changing DEBUG_$(shell whoami) to DEBUG_${USERNAME} in that
makefile help you?
(Assignee)

Comment 8

9 years ago
Created attachment 340483 [details] [diff] [review]
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.
Attachment #340483 - Flags: review?(julien.pierre.boogz)
(Assignee)

Comment 9

9 years ago
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

9 years ago
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 .

Updated

9 years ago
Attachment #340483 - Flags: review?(julien.pierre.boogz) → review+
Comment on attachment 340476 [details] [diff] [review]
change to my current userid

r=nelson
Attachment #340476 - Flags: review?(nelson) → review+
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
Attachment #340279 - Attachment description: remove DEBUG_nelsonb from NSS sources → remove DEBUG_nelsonb from NSS sources (checked in)

Comment 13

9 years ago
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 ?
(Assignee)

Comment 14

9 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Summary: Don't define or use DEBUG_$(shell whoami). → Change hyphens to underscores in DEBUG_$(shell whoami).
Target Milestone: --- → 3.12.2
You need to log in before you can comment on or make changes to this bug.