551345 - Need a workaround for global environment variables on Windows CE

Status: RESOLVED WONTFIX

(Core :: General, defect)

Description

[Alex Pakhotin (:alexp)]

Windows CE does not support environment variables, so it's difficult to use features, which rely on those, for example PR_LOG functionality widely used in NSPR.

Comment 1

[Alex Pakhotin (:alexp)]

Attachment: Use registry to define environment variables

Comment 2

[Doug Turner (:dougt)]

Comment on attachment 431518 [details] [diff] [review]
Use registry to define environment variables

+  if (result == ERROR_SUCCESS) {

return early and indent less.

+    for (DWORD dwIndex = 0; result == ERROR_SUCCESS; dwIndex++) {

change this for into a while.  It will be easier to read.

how does this impact performance?  Is a text file as good?

Comment 3

[Alex Pakhotin (:alexp)]

(In reply to comment #2)
> how does this impact performance?  Is a text file as good?

I'm pretty sure that checking registry must be faster than a file access, especially on WinMo, where as we know file system performance is not very good.

I haven't done complete tests by reading a lot of different files and different registry keys, but here are the results of a simple test, which hits the registry and file system 10000 times trying to open a non-existing registry key and a non-existing file (usec = microseconds, msec = milliseconds):
 open non-existing registry key:  76.90 usec avg, 769 msec total
 open non-existing file using PR_Open: 767.20 usec avg, 7672 msec total
 open non-existing file using CreateFile: 687.80 usec avg, 6878 msec total

It shows that at least in this case (most common, as we probably won't have any environment variables by default), registry is ~10 times faster than the file system, and the check takes less than 100 microseconds.

Comment 4

[Alex Pakhotin (:alexp)]

Attachment: Patch v2

Code review fixes.

Comment 5

[Doug Turner (:dougt)]

Comment on attachment 431780 [details] [diff] [review]
Patch v2

Aren't there system defines for this sort of thing?

+#define MAX_REG_NAME_LEN 64
+#define MAX_REG_VAL_LEN 256

+    dwIndex++;

Move this to inside RegEnumValue

Change the 'do' loop to a 'while' loop so that you can drop:

+  if (result != ERROR_SUCCESS)
+    return;

And nitpicking complete. :-)

Comment 6

[Alex Pakhotin (:alexp)]

(In reply to comment #5)
> Aren't there system defines for this sort of thing?
> 
> +#define MAX_REG_NAME_LEN 64
> +#define MAX_REG_VAL_LEN 256

Registry names and values are not limited in length, so there are no such system defines.
What we could do though - rename these to ENV_MAX_NAME_LEN and ENV_MAX_VAL_LEN, and use them in other functions in this module instead of the explicit 256.

> Change the 'do' loop to a 'while' loop so that you can drop:
> 
> +  if (result != ERROR_SUCCESS)
> +    return;

We still need an "if" to make sure RegCloseKey is called only when RegOpenKeyEx succeeds.

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 431780 [details] [diff] [review]
Patch v2

discussed on irc.