Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

Vadim Zeitlin-3
On 17 Jan 2006 09:47:04 -0000 [hidden email] wrote:

> Update of /pack/cvsroots/wxwidgets/wxWindows/src/msw
> In directory sunsite.dk:/tmp/cvs-serv28197/src/msw
>
> Modified Files:
> stdpaths.cpp
> Log Message:
> Applied patch [ 1385822 ] fixed stdpaths get folder path on WinCE
> Vince Harron
>
>
> Index: stdpaths.cpp
> ===================================================================
> RCS file: /pack/cvsroots/wxwidgets/wxWindows/src/msw/stdpaths.cpp,v
> retrieving revision 1.9
> retrieving revision 1.10
> diff -u -b -r1.9 -r1.10
> --- stdpaths.cpp 2005/06/15 23:44:18 1.9
> +++ stdpaths.cpp 2006/01/17 09:46:27 1.10
> @@ -106,35 +106,45 @@
>  
>  static void ResolveShellFunctions()
>  {
> -    // don't give errors if the functions are unavailable, we're ready to deal
> -    // with this
> -    wxLogNull noLog;
> -
>      // start with the newest functions, fall back to the oldest ones
> -
> +#ifdef __WXWINCE__
> +    wxString shellDllName(_T("coredll"));
> +#else
>      // first check for SHGetFolderPath (shell32.dll 5.0)
> -    wxDynamicLibrary dllShell32(_T("shell32"));
> -    if ( !dllShell32.IsLoaded() )
> +    wxString shellDllName(_T("shell32"));
> +#endif
> +
> +    wxDynamicLibrary dllShellFunctions( shellDllName );
> +    if ( !dllShellFunctions.IsLoaded() )
>      {
> -        wxLogTrace(TRACE_MASK, _T("Failed to load shell32.dll"));
> +        wxString traceMessage = wxString::Format( _T("Failed to load %s.dll"), shellDllName );
> +        wxLogTrace(TRACE_MASK, traceMessage );
>      }

 Sorry, what's exactly the point of the above change?

 Thanks,
VZ


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

Julian Smart
Hi,

At 12:02 17/01/2006, you wrote:
>  Sorry, what's exactly the point of the above change?

On WinCE, SHGetSpecialFolderPath is in coredll.dll, not in shell32.dll.

Regards,

Julian




---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re[2]: Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

Vadim Zeitlin-3
On Tue, 17 Jan 2006 13:53:43 +0000 Julian Smart <[hidden email]> wrote:

JS> At 12:02 17/01/2006, you wrote:
JS> >  Sorry, what's exactly the point of the above change?
JS>
JS> On WinCE, SHGetSpecialFolderPath is in coredll.dll, not in shell32.dll.

 No, sorry, I'm afraid I wasn't clear enough. I was asking about
this particular chunk which changed

    wxLogTrace(xxx, "foo %s", ...);

into

    wxString tempstring;
    tempstring.Printf("foo %s", ...);
    wxLogTrace(xxx, tempstring);

or something like that. This is not ugly but also potentially fatal if
tempstring contains (another) "%".

 Thanks,
VZ


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re[2]: Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

Julian Smart
At 15:13 17/01/2006, you wrote:

>  No, sorry, I'm afraid I wasn't clear enough. I was asking about
>this particular chunk which changed
>
>     wxLogTrace(xxx, "foo %s", ...);
>
>into
>
>     wxString tempstring;
>     tempstring.Printf("foo %s", ...);
>     wxLogTrace(xxx, tempstring);
>
>or something like that. This is not ugly but also potentially fatal if
>tempstring contains (another) "%".

Sorry, I still don't understand what the problem is, though the code
is missing a (const wxChar*) cast within the Format.

Regards,

Julian




---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re[3]: Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

Vadim Zeitlin-3
On Tue, 17 Jan 2006 15:35:06 +0000 Julian Smart <[hidden email]> wrote:

JS> At 15:13 17/01/2006, you wrote:
JS> >  No, sorry, I'm afraid I wasn't clear enough. I was asking about
JS> >this particular chunk which changed
JS> >
JS> >     wxLogTrace(xxx, "foo %s", ...);
JS> >
JS> >into
JS> >
JS> >     wxString tempstring;
JS> >     tempstring.Printf("foo %s", ...);
JS> >     wxLogTrace(xxx, tempstring);

 To be more precise, the change is here

http://cvs.wxwidgets.org/viewcvs.cgi/wxWidgets/src/msw/stdpaths.cpp.diff?r1=1.9&r2=1.10

at the end of the first yellow chunk.

JS> >or something like that. This is not ugly but also potentially fatal if
JS> >tempstring contains (another) "%".
JS>
JS> Sorry, I still don't understand what the problem is,

 The problem I spoke of was that this is totally unnecessary. Why create
the temp string? It's just ugly.

JS> though the code is missing a (const wxChar*) cast within the Format.

 And -- thanks for noticing it -- this is much more serious, of course, as
currently it would just crash. A .c_str() should be definitely added to
shellDllName. But IMO the 2 lines should be folded back into one, I see no
reason for a temporary variable here.

 Thanks,
VZ


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re[3]: Re: CVS: [JS] wxWindows/src/msw stdpaths.cpp,1.9,1.10

Julian Smart
At 16:11 17/01/2006, you wrote:
>  The problem I spoke of was that this is totally unnecessary. Why create
>the temp string? It's just ugly.

Good point, now corrected, thanks.

Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]