Comment 32 for bug 23369

Comment on attachment 117543
patch (remove printf, and unused definition)

+ if (!enabled) {
+ SYSPREF_LOG(("...Config config.system-pref is disabled\n"));
+ return NS_ERROR_FAILURE;
+ }

is this really an error condition to Observe()? Just return NS_OK (I think
Observe() results are ignored anyway, but just for good measure..)

+struct SysPrefItem {
+ const char *prefName;
+ PRInt32 prefType;
+};

why store the preftype? you can get it by calling getPrefType()

+static SysPrefItem sysPrefList[] = {
+#include "sys_pref_list.inc"
+ {nsnull, nsnull},
+};

globals (Even when static) should be prefixed with "g" as in "gSysPrefList" -
and you don't need to null terminate it (see below)

also, lists like this should be "const" so they get stored in a read-only
section of memory.

Loops like this:
+ while (sysPrefList[prefIndex].prefName) {
+ ReadSystemPref(prefIndex);
+ SYSPREF_LOG(("Add Listener on %s\n",
sysPrefList[prefIndex].prefName));
+ sysPrefBranchInternal->AddObserver(sysPrefList[prefIndex].prefName,
+ this, PR_TRUE);
+ ++prefIndex;
+ }

you should be using

#define SYSPREFLIST_LENGTH (sizeof(syspreflist) / sizeof(syspreflist[0]))
for (i=0; i<SYSPREFLIST_LENGTH; i++)

actually, it looks like you can share the pref list between your
prefNameMapping (which should have been gPrefNameMapping) and your sysPrefList
maps - why not just have one big map, instead of two maps that you have to keep
in sync?