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.
> 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.
> > + 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.
> -----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