cristianadam
11th September 2002 23:09 UTC

nsis 2.0 alpha 7 bug
Hi all,

On my machine running Windows XP nsis dies when launched.

I've tracked it down to this:

// Added by Ximon Eighteen 5th August 2002
#ifdef NSIS_CONFIG_PLUGIN_SUPPORT
void CEXEBuild::build_plugin_table(void)
{
plugin_used = false;
uninst_plugin_used = false;
char* nsisdir = definedlist.find("NSISDIR");
if (nsisdir)
{
char* searchPath = new char [strlen(nsisdir)+6];
if (searchPath)
{
wsprintf(searchPath,"%s\\plugins",nsisdir);
INFO_MSG("\nProcessing plugin dlls: \"%s\\*.dll\"\n",searchPath);
m_plugins.FindCommands(searchPath,display_info?true:false);
delete[] searchPath; // <<<<< THIS IS THE LINE
}
}
}

I've seen that operator delete is redefined in ResourceEditor.cpp using malloc/free functions, and no operator new[] and operator delete[] is defined ... this doesn't seem good.

Ofcorse if I undefine the NSIS_CONFIG_PLUGIN_SUPPORT everything seams to be ok.

I'm posting this here because I don't have a sourceforge account, yet.

Best regards,
Cristian.

Sunjammer
12th September 2002 00:11 UTC

Sounds like you may have found whats killing my installation of NSIS at work (my home copy works fine so this has been confusing me)... KiCHiK -- you there (hint hint) ? :D

CodeSquid
12th September 2002 10:31 UTC

Ah! The allocated memory is just a little bit too small.

Reason:
-------

char* searchPath = new char [strlen(nsisdir)+6];
...
wsprintf(searchPath,"%s\\plugins",nsisdir);

You're copying more bytes then allocated before, thus you overwrite memory that could belong to anything else.

Historical analysis:
--------------------

Plugins were stored in \bin, but now in \plugins, which is 4 bytes longer. Apparently someone did change the code without checking for sideeffects :eek: Shame on you! :D

Conclusion:
-----------

The new line should be like this:
char* searchPath = new char [strlen(nsisdir)+10];

This bug is most likely the reason for all the crashes some people still get. I would advise to check for similar bugs.

Sunjammer
12th September 2002 10:36 UTC

Ah not my bug then :P (cool)

Joost Verburg
12th September 2002 11:38 UTC

kickik made a bugfix in the CVS version a few weeks ago. But he changed it to 9. :igor:

// Added by Ximon Eighteen 5th August 2002
#ifdef NSIS_CONFIG_PLUGIN_SUPPORT
void CEXEBuild::build_plugin_table(void)
{
  plugin_used = false;
  uninst_plugin_used = false;
  char* nsisdir = definedlist.find("NSISDIR");
  if (nsisdir)
  {
    char* searchPath = new char [strlen(nsisdir)+9];
    if (searchPath)
    {
      wsprintf(searchPath,"%s\\plugins",nsisdir);
      INFO_MSG("\nProcessing plugin dlls: \"%s\\*.dll\"\n",searchPath);
      m_plugins.FindCommands(searchPath,display_info?true:false);
      delete[] searchPath;
    }
  }
}

Sunjammer
12th September 2002 11:44 UTC

"%s\\plugins",nsisdir -- so space required should be strlen(nsisdir)+one for the \\ (which is actually just \) and then 7 for "plugins" and another one for the null terminator..
i.e. strlen(nsisdir)+9.

kichik
12th September 2002 11:48 UTC

It should be nine (\\ - 1, p - 2, l - 3, u - 4, g - 5, i - 6, n - 7, s - 8, \0 - 9). Rainwater made the bug, and eccles fixed it.

And as for the original "solution", I have checked this when I added it. It does use my new operator even with new[].

rainwater
12th September 2002 12:33 UTC

"plugins" was "bin" before. Therefore, when I changed it to plugins, the allocation was incorrect.