Memnarch / Delphinus

An alternative Packagemanager for the Delphi-IDE
Mozilla Public License 2.0
235 stars 64 forks source link

Possible access violation when installing. #5

Closed NickRing closed 9 years ago

NickRing commented 9 years ago

In TDNRegistryEnvironmentOptions.Create there are two external calls:

FRegistryOptions := GetRegistryOptionsObject(GetEnvironmentOptionObject(), ARegistryKey);
FRegistry := GetRegistryOptionsMemIni(FRegistryOptions);

If GetRegistryOptionsObject returns nil, it is passed in to GetRegistryOptionsMemIni. However there is no check within the method to see if the object passed in is nil, so it will access violate when the following line is executed:

begin
  Result := nil;

  LType := LContext.GetType(ARegOption.ClassType);

Where ARegOption is the passed in object. The following line should be added after Result := nil;

if not assigned(ARegOption) then
  Exit;

Using XE7.1

Memnarch commented 9 years ago

Definetly something to improve. I'll do that. Did it throw an AV? (if it does we've got a problem)

NickRing commented 9 years ago

It did throw an AV while trying to install :-(

I finally got around to test Delphinus around midnight (just the best time to, right?) and it wasn't installing because of the AV and by commenting out code, it lead me to the above situation. I haven't had time to read and understand what it was trying to do, sorry :(

Memnarch commented 9 years ago

well, that code is quite critical. It gives me access to the seperated environment*options(library-path) for each platform (win32;win64;OSX32) seperately(the OTA-API is broken and does not work for this anymore). If this code completely fails, i cant configure the environment when you install a package. For delphi XE7, it should try the above code 3 times (Win32, Win64, OSX32). Does each of these fail? or just a specific one?

and i hope GetEnvironmentOptionObject() returns something?

NickRing commented 9 years ago

Give me a couple of days and I will try to debug it (full-on weekend :( )

Memnarch commented 9 years ago

thank you, that's great :)

NickRing commented 9 years ago

Found the issue...

Delphinus assumes that is the registry key exists (in TDNEnvironmentOptionsService.LoadPlatforms), that the platform is available. This is not the case for me, as I didn't install the OSX32 platform, as I don't have any OSX32 devices. Because I haven't installed the OSX32 platform, Delphinus is unable to retrieve the OSX32 library path (in GetRegistryOptionsObject).

So there is two problems I see:

  1. The install process for XE7 is broken, in that it add keys for non-existent platforms. Possibility of getting fix, none.
  2. Delphinus doesn't handle missing platforms. Possibility of getting fixed, fairly high.
NickRing commented 9 years ago

Created a pull request to overcome this issue.

Memnarch commented 9 years ago

merged it. Thanks for providing the fix :)