[perl #123658] [PATCH] stop checking the Win32 registry if *"/Software/Perl" doesn't exist

On Fri Jan 23 10:26:04 2015, bulk88 wrote:
> AS PDK's GUI debugger installs itself as value name "PERL5DB" and
> "BEGIN {require q<C:\Program Files\ActiveState Perl Dev Kit
> 9.1\bin\lib\PerlDB.pl>}" as the value (and that is the only value I
> have in "HKLM/SOFTWARE/Perl"). I dont think the registry lookup
> feature can be removed.

Maybe we could add a build define to disable it.

As to your patch:

+	if (!ptr) {
+	    ptr = (char*)HKLM_Perl_hnd;
+	    if (ptr)
+		ptr = get_regstr_from((HKEY)ptr, valuename, svp);
+	    /* else pass through NULL */

is this cast assignment to ptr an optimization for some compiler?

I don't think it's acceptable code.

I'm kind of surprised you didn't structure it closer to the original, something like:

+    if (HKCU_Perl_hnd)
+	ptr = get_regstr_from(HKCU_Perl_hnd, valuename, svp);
+    if (!ptr && HKLM_Perl_hnd)
+	 ptr = get_regstr_from((HKLM_Perl_hnd, valuename, svp);
+
+    return ptr;

which is a lot easier to read, and should be about the same or smaller code size and speed.

+    /* handles might be NULL, RegCloseKey then returns ERROR_INVALID_HANDLE
+       but no point of checking and we can't die() at this point */
+    RegCloseKey(HKLM_Perl_hnd);
+    RegCloseKey(HKCU_Perl_hnd);
+    /* the handles are in an undefined state until the next PERL_SYS_INIT3 */

My main problem with this is that it will increase noise in tools that
check for bad API calls.

Adding conditional checks here for a function (Perl_win32_term()) that 
executes *once* per perl invocation doesn't seem like a huge price to pay 
for reducing that noise.

Tony

---
via perlbug:  queue: perl5 status: open
https://rt.perl.org/Ticket/Display.html?id=123658

• [perl #123658] [PATCH] stop checking the Win32 registry if *"/Software/Perl" doesn't exist by Steve Hay via RT
• [perl #123658] [PATCH] stop checking the Win32 registry if *"/Software/Perl" doesn't exist by Tony Cook via RT