Closed p5pRT closed 8 years ago
Need #.
On Fri Jan 23 00:26:22 2015\, bulk88 wrote:
This is a bug report for perl from bulk88@hotmail.com\, generated with the help of perlbug 1.40 running under perl 5.21.4.
----------------------------------------------------------------- [Please describe your issue here]
Need #.
Patch attached. Some benchmarks I did. This patch probably saves 1 to 2 ms out of 10 to 30 ms per process start.
C:\sources\perlbench>perl perlbench-run -c 100000 newreg=C:\perl521\newreg\bin\ perl.exe oldreg=C:\perl521\oldreg\bin\perl.exe new2=C:\perl521\newreg\bin\perl. exe old2=C:\perl521\oldreg\bin\perl.exe newreg) perl-5 version = 5.021008 path = C:\perl521\newreg\bin\perl.exe
oldreg) perl-5 version = 5.021008 path = C:\perl521\oldreg\bin\perl.exe
new2) perl-5 version = 5.021008 path = C:\perl521\newreg\bin\perl.exe
old2) perl-5 version = 5.021008 path = C:\perl521\oldreg\bin\perl.exe
newreg oldreg new2 old2 ------ ------ ---- ---- arith/mixed 100 102 98 103 INVALID arith/trig 100 99 100 98 OLD array/copy 100 94 106 102 OLD array/foreach 100 103 99 99 NEW array/index 100 106 98 97 INVALID array/pop 100 103 100 108 NEW array/shift 100 110 105 106 NEW array/sort-num 100 100 101 97 OLD array/sort 100 100 96 95 OLD call/0arg 100 105 97 105 NEW call/1arg 100 97 95 96 INVALID call/2arg 100 94 102 97 OLD call/9arg 100 99 98 97 OLD call/empty 100 100 105 100 INVALID call/fib 100 96 100 94 OLD call/method 100 100 100 97 OLD call/wantarray 100 100 99 99 INVALID hash/copy 100 94 94 106 INVALID hash/each 100 95 102 95 OLD hash/foreach-sort 100 100 98 108 NEW hash/foreach 100 112 99 116 NEW hash/get 100 101 92 104 NEW hash/set 100 97 101 100 OLD loop/for-c 100 120 96 118 NEW loop/for-range-const 100 94 102 97 OLD loop/for-range 100 94 99 97 OLD loop/getline 100 108 109 94 INVALID loop/while-my 100 104 101 104 NEW loop/while 100 101 97 96 INVALID re/const 100 99 102 103 INVALID re/w 100 104 101 101 NEW startup/fewmod 100 144 - 143 NEW startup/lotsofsub - - - - startup/noprog 100 109 90 90 NEW string/base64 100 105 91 98 NEW string/htmlparser 100 101 95 97 NEW string/index-const 100 102 99 102 NEW string/index-var 100 135 110 127 NEW string/ipol 100 99 99 101 INVALID string/tr 100 93 105 88 OLD
AVERAGE 100 103 99 102
Results saved in file:///C|/sources/perlbench/benchres-015/index.html
C:\sources\perlbench>
I judged the numbers by eye\, INVALID means the test is contradictory on time\, and is measuring something irrelavent
13 OLD IS FASTER 16 NEW IS FASTER 10 JITTER MAKES IT INVALID
C:\sources\perlbench>perl -MBenchmark=:all\,:hireswallclock -E"cmpthese(10000\, {' old' => sub {system'C:\perl521\oldreg\bin\perl.exe -e\"0\"'}\, 'new' => sub {syst em'C:\perl521\newreg\bin\perl.exe -e\"0\"'}})" Rate old new old 1481/s -- -9% new 1624/s 10% --
C:\sources\perlbench>
I dont think I trust that number\, with perl taking 1/1481=0.000675 seconds or 0.6 ms to start up and exit. But it does show an improvement.
The procmon logs were generated by running "C:\perl521\oldreg\bin\perl.exe -e"print $ENV{PERLNOTHERE}.$ENV{PERLHERE}"" and "C:\perl521\newreg\bin\perl.exe -e"print $ENV{PERLNOTHERE}.$ENV{PERLHERE}"" from cmd.exe shell. I added PERLHERE to my registry for testing.
The 4 calls to "RegQueryValue" are because RegQueryValueExA calls RegQueryValueExW once to get the length of the utf16 data\, it them allocs a temp buffer\, then calls RegQueryValueExW again\, then does utf16 to ANSI conversion\, then if a user supplied buffer exists ("LPBYTE lpData")\, it copies its buffer over\, and finally it writes the ANSI length count into the user supplied "LPDWORD lpcbData". Since Perl calls RegQueryValueExA twice\, once to get the length\, then allocate Perl buffer\, then calls RegQueryValueExA again\, the procmon log shows 4 "RegQueryValue" calls. Turning the value name into utf16 ourselves in get_regstr (but only if 1 or 2 of the root handles are true)\, since remember both HKCU and HKLM SOFTWARE/Perl might exist in worst case scenario\, so the value name should not be converted by Perl once for HKCU and once for HKLM\, and converting back the value data buffer back to ANSI ourselves in get_regstr_from would be the smarter solution but that is another day\, for another patch (it also brings up alloca vs Newx and SAVEFREEPV\, and should Perl use "MultiByteToWideChar(CP_ACP" or the more efficient "RtlMultiByteToUnicodeN" https://msdn.microsoft.com/en-us/library/windows/hardware/ff553113%28v=vs.85%29.aspx which is what all the "A" functions use on Win NT series OSes\, or should all Win32 Perl OS APIs accept UTF8? and I dont want to open any of those things for discussion in this ticket). Plus since the env vars themselves are stored in utf16 in the PEB on win nt\, going from ansi to utf16 ASAP in perl is preferable. If HKCU/HKLM SOFTWARE/Perl exists\, getting a "found" in the registry is extremely rare compared to the number of not founds. After this patch\, a notfound is 1 syscall\, a found is 4\, so patch at making notfound scenario faster is a much bigger improvement than making the rare "found" scenario faster. The found scenario is still faster than before since we didn't have to open and close the root handle to */SOFTWARE/Perl first.
new from first call to lookup PERL_UNICODE "7:05:08.3765673 AM"\,"perl.exe"\,"3980"\,"3084"\,"RegQueryValue"\,"HKLM\SOFTWARE\Perl\PERL_UNICODE"\,"NAME NOT FOUND"\,"Length: 144" to last (and first) call to lookup PERLLIB "7:05:08.3768070 AM"\,"perl.exe"\,"3980"\,"3084"\,"RegQueryValue"\,"HKLM\SOFTWARE\Perl\PERLLIB"\,"NAME NOT FOUND"\,"Length: 144"
.3768070-.3765673=0.0002397\, .2 ms
old from first call to lookup PERL_UNICODE "7:04:02.5810787 AM"\,"perl.exe"\,"312"\,"3188"\,"RegOpenKey"\,"HKCU\Software\Perl"\,"NAME NOT FOUND"\,"Desired Access: Read" to last call to lookup PERLLIB "7:04:02.5819799 AM"\,"perl.exe"\,"312"\,"3188"\,"RegCloseKey"\,"HKLM\SOFTWARE\Perl"\,"SUCCESS"\,""
.5819799-.5810787=0.0009012 .9ms
There is some overhead in using procmon/any syscall tracer so i'll say its 1-3 ms saved per process start. Remember I have a HKLM/SOFTWARE/Perl on my normal dev machine\, my alternate machine has no SOFTWARE/Perl in either HKCU or HKLM\, so it would be even faster.
-- bulk88 ~ bulk88 at hotmail.com
[Apologies for Outlook's top-posting...]
At $work we actually remove the registry lookup of %ENV stuff altogether to prevent other installations of Perl on customers' machines from interfering with our own software's installation of Perl. The attached (very minimal) patch does the business.
I would personally be very happy to see the %ENV registry lookup disappear completely\, and I vaguely recall JDB saying the same years ago. If that's correct (apologies if I've misremembered!) then it's presumably not important to ActivePerl\, so if you're correct in your patch comment that "ActiveState is the only known major user of the Software/Perl reg key" then maybe we can indeed get rid of it.
That's probably a contentious change\, though\, so I'm just too late in thinking of this for 5.22. Could we add a deprecation notice in 5.22 and drop the feature for 5.23?
-----Original Message----- From: bulk88 via RT [mailto:perlbug-followup@perl.org] Sent: 23 January 2015 11:45 Cc: perl5-porters@perl.org Subject: [perl #123658] [PATCH] stop checking the Win32 registry if *"/Software/Perl" doesn't exist
On Fri Jan 23 00:26:22 2015\, bulk88 wrote:
This is a bug report for perl from bulk88@hotmail.com\, generated with the help of perlbug 1.40 running under perl 5.21.4.
----------------------------------------------------------------- [Please describe your issue here]
Need #.
Patch attached. Some benchmarks I did. This patch probably saves 1 to 2 ms out of 10 to 30 ms per process start.
The RT System itself - Status changed from 'new' to 'open'
On Fri Jan 23 04:03:26 2015\, Steve.Hay@verosoftware.com wrote:
I would personally be very happy to see the %ENV registry lookup disappear completely\, and I vaguely recall JDB saying the same years ago. If that's correct (apologies if I've misremembered!) then it's presumably not important to ActivePerl\, so if you're correct in your patch comment that "ActiveState is the only known major user of the Software/Perl reg key" then maybe we can indeed get rid of it.
That's probably a contentious change\, though\, so I'm just too late in thinking of this for 5.22. Could we add a deprecation notice in 5.22 and drop the feature for 5.23?
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.
-- bulk88 ~ bulk88 at hotmail.com
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
On Mon Jan 26 15:06:22 2015\, tonyc wrote:
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.
If removing it completely is not an option then I would be very much in favour of a build option to disable it. Perhaps even have the option off by default since most people probably don't have any need for it.
On Mon Jan 26 15:06:22 2015\, tonyc wrote:
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.
I added the feature\, IDK what the internal C name should be of the C macro control the feature. USE_* seems to be for Configure vars\, this windows only build option is not the unix COnfigure script\, WIN32_* shows its not a Configure var. If "USE_NO_REGISTRY" is okay to use as a name\, even though it makes no sense on non-Win32 as to what it is to do\, I can get rid of the WIN32_NO_REGISTRY internal C name and have it be called USE_NO_REGISTRY to the user\, and USE_NO_REGISTRY in the C code. Or do you think the present 2 name way as I did it in the commit is fine?
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;
You code above has var ptr uninitialized in a particular branch permutation. I removed the first void * statement "ptr = HKCU_Perl_hnd;" usage on the 1st branch\, I couldn't on the 2nd test since otherwise I'd have to introduce a "ptr = NULL;" and avoiding "ptr = NULL;" statement was the point of writing it with void *s. AFAIK only clang knows that a false register contains the value NULL and can optimize away the assignment of NULL and generation of NULL ("xor eax\, eax").
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
AppVerifier doesn't complain/notice (if I managed to get it to run/instrument the right binary\, it was a month ago month I tested it). VC Debugger also doesn't give me popups about bad handles.
-- bulk88 ~ bulk88 at hotmail.com
On Mon Jan 26 15:06:22 2015\, tonyc wrote:
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.
I added the feature\, IDK what the internal C name should be of the C macro control the feature. USE_* seems to be for Configure vars\, WIN32_* shows its not a Configure var. If "USE_NO_REGISTRY" is okay to use as a name\, even though it makes no sense on non-Win32\, I can get rid of the WIN32_NO_REGISTRY internal C name and have it be called USE_NO_REGISTRY to the user\, and USE_NO_REGISTRY in the C code.
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;
You code above has var ptr uninitialized in a particular branch permutation. I removed the first void * statement "ptr = HKCU_Perl_hnd;" usage on the 1st branch\, I couldn't on the 2nd test since otherwise I'd have to introduce a "ptr = NULL;" and avoiding "ptr = NULL;" statement was the point of writing it with void *s. AFAIK only clang knows that a false register contains the value NULL and can optimize away the assignment of NULL.
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
AppVerifier doesn't complain/notice (if I managed to get it to run/instrument the right binary\, it was a ago month I tested it). VC Debugger also doesn't give me popups about bad handles.
-- bulk88 ~ bulk88 at hotmail.com
I also have a dream\, so that with-registry-on builds\, do not use advapi32 for anything except GetUserNameA\, the registry lookups instead use Native API\, from ntdll.dll. ntdll.dll is in every process\, advapi32 is optional. Advapi32's registry API allows you to read/write/modify the registry of another windows computer over a LAN\, as easily as your own local registry. But nobody except for MSCE IT depts will ever use remote registry service and DCOM\, and Perl has no business using a foreign machine's registry for its ENV var overrides. So the registry lookups go through ntdll.dll\, the GetUserNameA stays as part of advapi32 and runs through a delay load of advapi32. (GetUserNameA is a RPC call to LSASS service using NT LPC ports in every implementation\, there is no Native [user mode] (ntdll/ntoskrnl) API\, the kernel driver API equivalent for GetUserNameA in ksecdd.sys also does LPC port RPC to LSASS\, so there is no advantage in not using the public api and ksecdd.sys is not accessible from user mode (not in the SSDT)). GetUserNameA is a simple stub in advapi32 for secur32.dll's GetUserNameExA function. advapi32 itself delay loads secur32.dll if you call advapi32's GetUserNameA.
The shim is
BOOL __stdcall GetUserNameA(LPSTR lpBuffer\, LPDWORD pcbBuffer) { return GetUserNameExA((EXTENDED_NAME_FORMAT)65538\, lpBuffer\, pcbBuffer); }
Looking at my winxp secur32.dll\, I see secur32.dll loads advapi32.dll\, so whether you delay load with Perl on GetUserNameA/advapi32.dll or GetUserNameExA/secur32.dll\, when you call either both advapi32 and secur32 will wind up being loaded into the perl process in any case\, the point is to keep advapi32/secur32.dll/rpcrt4.dll out of the process until you call getlogin. I need to do some research if calling GetUserNameExA/secur32.dll will trigger advapi32 to instantly delay load/bind itself to secur32 or not (perl->DelayLoadDLL@perl522.dll->GetUserNameExA@secur32.dll->FooBar@advapi32.dll->DelayLoadDLL@advapi32.dll->Baz@secur32.dll). If advapi32.dll stays unbound and unaware that secur32.dll is in the process even though secur32.dll is what loaded advapi32.dll into the process\, some CPU and maybe a COW mem page (4KB) representing the delay import table in advapi32.dll is saved. Static dynamic linking is more efficient than the VC delay loader code I believe. For example the VC delay loader uses GetProcAddress\, and each call to GetProcAddress has to get the DLL Loader mutex\, while using static dynamic linking the lock is obtained once inside LoadLibrary() in kernel32/Ldr*() in ntdll.
-- bulk88 ~ bulk88 at hotmail.com
On Thu May 28 12:49:20 2015\, bulk88 wrote:
Looking at my winxp secur32.dll\, I see secur32.dll loads advapi32.dll\, so whether you delay load with Perl on GetUserNameA/advapi32.dll or GetUserNameExA/secur32.dll\, when you call either both advapi32 and secur32 will wind up being loaded into the perl process in any case\, the point is to keep advapi32/secur32.dll/rpcrt4.dll out of the process until you call getlogin. I need to do some research if calling GetUserNameExA/secur32.dll will trigger advapi32 to instantly delay load/bind itself to secur32 or not (perl->DelayLoadDLL@perl522.dll-
GetUserNameExA@secur32.dll->FooBar@advapi32.dll- DelayLoadDLL@advapi32.dll->Baz@secur32.dll). If advapi32.dll stays unbound and unaware that secur32.dll is in the process even though secur32.dll is what loaded advapi32.dll into the process\, some CPU and maybe a COW mem page (4KB) representing the delay import table in advapi32.dll is saved. Static dynamic linking is more efficient than the VC delay loader code I believe. For example the VC delay loader uses GetProcAddress\, and each call to GetProcAddress has to get the DLL Loader mutex\, while using static dynamic linking the lock is obtained once inside LoadLibrary() in kernel32/Ldr*() in ntdll.
The USERNAME env var could be used for getlogin instead of advapi32/secur32\, but there is a tiny risk someone will mess with the env var in the parent process of perl (a cmd.exe process)\, or in the perl process at runtime. The USERNAME env var is created in Winlogon.exe\, specifically in userenv.dll\, userenv.dll calls GetUserNameExW from secur32.dll. No undocumented APIs there for me to try to use. USERNAME env var is then inherited down process tree. Both taskman and explorer\, when launches from winlogon/ctl-alt-del screen get USERNAME. Service processes never have USERNAME in their env vars according to process explorer.
-- bulk88 ~ bulk88 at hotmail.com
On Thu May 28 10:14:04 2015\, bulk88 wrote:
On Mon Jan 26 15:06:22 2015\, tonyc wrote:
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.
I added the feature\, IDK what the internal C name should be of the C macro control the feature. USE_* seems to be for Configure vars\, WIN32_* shows its not a Configure var. If "USE_NO_REGISTRY" is okay to use as a name\, even though it makes no sense on non-Win32\, I can get rid of the WIN32_NO_REGISTRY internal C name and have it be called USE_NO_REGISTRY to the user\, and USE_NO_REGISTRY in the C code.
Thanks.
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;
You code above has var ptr uninitialized in a particular branch permutation. I removed the first void * statement "ptr = HKCU_Perl_hnd;" usage on the 1st branch\, I couldn't on the 2nd test since otherwise I'd have to introduce a "ptr = NULL;" and avoiding "ptr = NULL;" statement was the point of writing it with void *s. AFAIK only clang knows that a false register contains the value NULL and can optimize away the assignment of NULL.
Then initialize ptr at the declaration.
Please don't reduce readability for tiny space improvements.
+ /* 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
AppVerifier doesn't complain/notice (if I managed to get it to run/instrument the right binary\, it was a ago month I tested it). VC Debugger also doesn't give me popups about bad handles.
Ok.
Tony
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
-- bulk88 ~ bulk88 at hotmail.com
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
Eghh "[PATCH 2/2] add Win32 USE_NO_REGISTRY build option "'s commit message has a line of "XXXXXXXXXXXXXXXXXXXXXXX" I left in by accident. I'll leave it to the committer to remove.
-- bulk88 ~ bulk88 at hotmail.com
On Thu Jul 09 03:17:40 2015\, bulk88 wrote:
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
Eghh "[PATCH 2/2] add Win32 USE_NO_REGISTRY build option "'s commit message has a line of "XXXXXXXXXXXXXXXXXXXXXXX" I left in by accident. I'll leave it to the committer to remove.
Bump.
-- bulk88 ~ bulk88 at hotmail.com
On Sun Aug 02 21:15:26 2015\, bulk88 wrote:
On Thu Jul 09 03:17:40 2015\, bulk88 wrote:
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
Eghh "[PATCH 2/2] add Win32 USE_NO_REGISTRY build option "'s commit message has a line of "XXXXXXXXXXXXXXXXXXXXXXX" I left in by accident. I'll leave it to the committer to remove.
Bump. Bump.
-- bulk88 ~ bulk88 at hotmail.com
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
You haven't addressed:
Please don't reduce readability for tiny space improvements.
get_regstr(const char *valuename\, SV **svp) { - char *str = get_regstr_from(HKEY_CURRENT_USER\, valuename\, svp); - if (!str) - str = get_regstr_from(HKEY_LOCAL_MACHINE\, valuename\, svp); + char *str; + if (HKCU_Perl_hnd) { + str = get_regstr_from(HKCU_Perl_hnd\, valuename\, svp); + if (!str) + goto try_HKLM; + } + else { + HKEY hkey; + try_HKLM: + hkey = HKLM_Perl_hnd; + if (hkey) + str = get_regstr_from(hkey\, valuename\, svp); + else + str = (char*)hkey; /* source of NULL without litteral NULL constant */ + } return str; }
Tony
On Sun Aug 16 18:30:18 2015\, tonyc wrote:
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
You haven't addressed:
Please don't reduce readability for tiny space improvements.
revised get_regstr patches attached
-- bulk88 ~ bulk88 at hotmail.com
On Wed Sep 30 03:27:41 2015\, bulk88 wrote:
On Sun Aug 16 18:30:18 2015\, tonyc wrote:
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
You haven't addressed:
Please don't reduce readability for tiny space improvements.
revised get_regstr patches attached
Bump.
-- bulk88 ~ bulk88 at hotmail.com
On Wed Sep 30 03:27:41 2015\, bulk88 wrote:
On Sun Aug 16 18:30:18 2015\, tonyc wrote:
On Thu Jul 09 03:01:53 2015\, bulk88 wrote:
Revised patches attached. Rebased and get_regstr was reworked to use 2 vars instead of 1.
You haven't addressed:
Please don't reduce readability for tiny space improvements.
revised get_regstr patches attached
Thanks\, applied as 0517ed3816767f5896256870b8cca4b856e4088a and 6937817d58b1688d689072cd112ed95fe62db2a7.
Tony
@tonycoz - Status changed from 'open' to 'resolved'
Migrated from rt.perl.org#123658 (status was 'resolved')
Searchable as RT123658$