Vladislav Vaintroub skrev:
> Hi Magnus,
> thanks a lot for review. I addressed you comments in
> http://lists.mysql.com/commits/54100
>> magnus: Somewhere you should set "path[0]= '\0'" to cover the case wher
>> no modules and no envvar.
>
> Yep, this is correct. Thanks.
ok
>
>> magnus: if no \ is found, you will add the full module dir
>> verbatim. But what about ending semicolon ?
>
> "if no \ is found" is not known to happen. If ever happens, which I doubt, I
> added code to append ".;" to the path , assuming current directory. An
> interesting case is using Unix path separator '/'. LoadLibrary will succeed
> with it, however Module32First()/Module32Next() returns Windows'y path with
> correct \ separators.
ok
>
>>> + strncat(path, size, module_dir);
>> magnus: ^ size should be last in 'strncat'
>
> Oops, thanks for catching this :)
:)
>
>>> +
>>> +#define MAX_SYMBOL_PATH 32768
>> magnus: quite big, but ok I guess.
>
> Yes, it is big enough to account for all situations , as many DLL's, long
> NT_SYMBOL_PATH etc. I put symbol_path into static storage, instead of
> stack, to exclude stack overflow possibility. Static storage class on the
> other hand should not be a problem - we never attempted to make this
> function thread-safe, and it has always been thread-unsafe, alone due to
> usage of unsafe dbghelp functions.
>
good, didn't think about "static"
Approved.
>
>> -----Original Message-----
>> From: Magnus.Svensson@stripped [mailto:Magnus.Svensson@stripped]
>> Sent: Monday, September 15, 2008 2:03 PM
>> To: Vladislav Vaintroub
>> Cc: commits@stripped
>> Subject: Re: bzr commit into mysql-5.1-bugteam branch (vvaintroub:2678)
>> Bug#35987
>
>
