Fix win32reg issues #5037 and #4702 #5038
Conversation
|
|
|
Hello! Github actions build windows installers that you can download and test your changes with. If you click on the actions tabs there is an artifacts dropdown where you can download |
|
Awesome. I have downloaded the windows installer, installed it on a clean VM, then run my test cases in an escript. I found one mistake, corrected it, and I believe the changes are good. |
| { | ||
| char* key = buf; | ||
|
|
||
| result = RegDeleteKey(rp->hkey, key); | ||
| reply(rp, result); | ||
| } |
There was a problem hiding this comment.
Edit: Just ignore me. I was watching the wrong code version and missed the change to win32reg:delete_key. I wrote:
I don't get how this could work (other than pure luck). The data buffer sent for CMD_DELETE_KEY does only contain one byte (the value 7). The pointer
bufhas been incremented (before theswitch) to point just beyond that first and only byte into unknown territory. Passing it as argument to any function is just wrong.According to
RegDeleteKeydocumentation, the second argumentlpSubKeyshould be the name of the subkey to be deleted.
There was a problem hiding this comment.
Maybe better (as @b-winkler proposed) to replace the broken delete_key with a new delete_subkey which is simpler to implement and does not have the quirk to invalidate the current key.
There was a problem hiding this comment.
This necessitates a change in the documentation as well, which is why I avoided it initially. If we're certain we prefer it, I can make that change. Let me know :)
There was a problem hiding this comment.
Well, if you have done a delete_key that actually works I'm fine with keeping it.
|
@reganheath I would prefer if you could rearrange the commits with one for each issue, Also you should describe the actual fix in the commit headline, like "Fix win32reg:value/2 for default value". If referring the github issue or pull request do that with GH-5027 and PR-5038. If you want, you could also rebase on maint ("git rebase --onto maint master") to get these fixes into OTP-24.1. |
|
... and if so, also click Edit and change this PR to be based on maint. |
sverker
force-pushed
the
5037_and_4702
branch
2 times, most recently
from
fa0c5e8
to
db14aa8
Compare
sverker
added
fix
testing
|
Rebased on maint and put into test. |
|
Added |
|
Sorry! I've been swamped with stuff recently. |
The function win32reg:delete_key() did not work as documented.
It did not delete the current registry key and instead always
returned {error,einval} instead of ok.
sverker
force-pushed
the
5037_and_4702
branch
from
858d1a8
to
cd7f4cf
Compare
|
Merged to maint for 24.1. |
Pull request to resolve #5037 and #4702 (issues with win32reg). I have to be honest, I found it very hard to debug and test this due to the complexity of building OTP on windows. I managed to build it in WSL, but of course this doesn't help as the code in Q is windows only.