#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
90 views
Skip to first unread message
wxTrac
May 25, 2012, 5:12:02 PM5/25/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Converting helpers is better then inlined casts because of:
1. They are smaller and simpler. It is easier to fit into 80 columns
source code width.
2. They are similar to wxConvertToGTK functions from
[http://trac.wxwidgets.org/browser/wxWidgets/trunk/include/wx/gtk/private.h
include/wx/gtk/private.h]
2. They have same suffixes as function argument names in Windows API
headers and MSDN.
3. They can guarantee that resulting type will be always same as declared
one.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Priority: normal | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Converting helpers is better then inlined casts because of:
1. They are smaller and simpler. It is easier to fit into 80 columns
source code width.
2. They are similar to wxConvertToGTK functions from
[http://trac.wxwidgets.org/browser/wxWidgets/trunk/include/wx/gtk/private.h
include/wx/gtk/private.h]
2. They have same suffixes as function argument names in Windows API
headers and MSDN.
3. They can guarantee that resulting type will be always same as declared
one.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338>
wxTrac
May 26, 2012, 7:33:48 AM5/26/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:1>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:
---------------------+------------------------------------------------------
Changes (by vadz):
* priority: normal => low
* status: new => confirmed
* patch: 1 => 0
Comment:
Unfortunately this doesn't work because `t_str()` returns a temporary
buffer in UTF-8 build and the temporary buffer is destroyed at the end of
expression, hence the pointer to its data becomes invalid so the patch
can't be applied as is. We could still have these as macros but this would
be much uglier, not sure if it's still worth it.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:1>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 0 | Blocking:
Keywords: | Blockedby:
---------------------+------------------------------------------------------
Changes (by vadz):
* priority: normal => low
* status: new => confirmed
* patch: 1 => 0
Comment:
Unfortunately this doesn't work because `t_str()` returns a temporary
buffer in UTF-8 build and the temporary buffer is destroyed at the end of
expression, hence the pointer to its data becomes invalid so the patch
can't be applied as is. We could still have these as macros but this would
be much uglier, not sure if it's still worth it.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:1>
wxTrac
May 26, 2012, 4:05:36 PM5/26/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:2>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:
* patch: 0 => 1
Comment:
Yes, wxScopedWCharBuffer is destroyed in these helper functions, but
fortunately wchar_t* strings is cached in wxString::m_convertedToWChar in
UTF-8 build, not in wxScopedWCharBuffer. Real cached string is stored in
wxString::ConvertedBuffer<wchar_t>::m_str and wxScopedWCharBuffer::m_data
stores only '''non-owned''' buffer. So wxString::t_str() is similar with
std::basic_string<>::c_str() even in UTF-8 build. So this patch is valid
and really works.
See r60116 and r45335 for details.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:2>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Changes (by kosenko):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
* patch: 0 => 1
Comment:
Yes, wxScopedWCharBuffer is destroyed in these helper functions, but
fortunately wchar_t* strings is cached in wxString::m_convertedToWChar in
UTF-8 build, not in wxScopedWCharBuffer. Real cached string is stored in
wxString::ConvertedBuffer<wchar_t>::m_str and wxScopedWCharBuffer::m_data
stores only '''non-owned''' buffer. So wxString::t_str() is similar with
std::basic_string<>::c_str() even in UTF-8 build. So this patch is valid
and really works.
See r60116 and r45335 for details.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:2>
wxTrac
May 27, 2012, 9:02:24 AM5/27/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:3>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:
* cc: vaclavslavik (added)
Comment:
I did think about it but IMO the fact that currently we always cache the
conversion results is just an optimization and could change in the future,
while if we add these functions we sign to always having these buffers
inside it forever because there is no way to implement them otherwise. I'm
not sure if this is wise. The only doubt in my mind is whether we're not
already committed to having this cache anyhow... I do know this is the
case for `char*` buffer but not certain about `wchar_t*`. Vaclav, do you
know/remember if we must have this buffer anyhow and so if it's safe to
return a pointer to it ("safe" in the sense of not regretting it in the
future, I do know that it is safe in the sense of not getting a dangling
pointer right now)?
Also, if it is safe, then we should just change `t_str()` itself to return
either `char*` or `wchar_t*` depending on the build, there is no need to
have a separate `wxConvertToLPCTSTR()` then.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:3>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Changes (by vadz):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
* cc: vaclavslavik (added)
Comment:
I did think about it but IMO the fact that currently we always cache the
conversion results is just an optimization and could change in the future,
while if we add these functions we sign to always having these buffers
inside it forever because there is no way to implement them otherwise. I'm
not sure if this is wise. The only doubt in my mind is whether we're not
already committed to having this cache anyhow... I do know this is the
case for `char*` buffer but not certain about `wchar_t*`. Vaclav, do you
know/remember if we must have this buffer anyhow and so if it's safe to
return a pointer to it ("safe" in the sense of not regretting it in the
future, I do know that it is safe in the sense of not getting a dangling
pointer right now)?
Also, if it is safe, then we should just change `t_str()` itself to return
either `char*` or `wchar_t*` depending on the build, there is no need to
have a separate `wxConvertToLPCTSTR()` then.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:3>
wxTrac
May 29, 2012, 1:25:56 PM5/29/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:4>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:
Replying to [comment:3 vadz]:
eventually go away or we'll at least have a mode to disable them. I don't
think it's wise to rely on them for new stuff and would very much prefer
not to do it and either not do anything or implement them as macros.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:4>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Comment(by vaclavslavik):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Replying to [comment:3 vadz]:
> Vaclav, do you know/remember if we must have this buffer anyhow
They're for backward compatibility only, IIRC, so presumably they'll
eventually go away or we'll at least have a mode to disable them. I don't
think it's wise to rely on them for new stuff and would very much prefer
not to do it and either not do anything or implement them as macros.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:4>
wxTrac
May 29, 2012, 2:38:23 PM5/29/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:5>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: confirmed
Priority: low | Milestone:
Vaclav, what about wxString::t_str() return type? Should it return in all
builds const wxChar* instead of wxScopedWCharBuffer for some builds.
{{{
#!cpp
#if wxUSE_UNICODE_UTF8
const wxScopedWCharBuffer t_str() const { return wc_str(); }
#elif wxUSE_UNICODE_WCHAR
const wchar_t* t_str() const { return wx_str(); }
#else
const char* t_str() const { return wx_str(); }
#endif
}}}
Note wxScopedCharTypeBuffer<wxChar> has cast operators only to wxChar* and
const wxChar*.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:5>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Comment(by kosenko):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Vaclav, what about wxString::t_str() return type? Should it return in all
builds const wxChar* instead of wxScopedWCharBuffer for some builds.
{{{
#!cpp
#if wxUSE_UNICODE_UTF8
const wxScopedWCharBuffer t_str() const { return wc_str(); }
#elif wxUSE_UNICODE_WCHAR
const wchar_t* t_str() const { return wx_str(); }
#else
const char* t_str() const { return wx_str(); }
#endif
}}}
Note wxScopedCharTypeBuffer<wxChar> has cast operators only to wxChar* and
const wxChar*.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:5>
wxTrac
Jun 1, 2012, 5:25:12 AM6/1/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:6>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: infoneeded_new
Priority: low | Milestone:
* status: confirmed => infoneeded_new
Comment:
Sorry if I'm missing something but what does the second patch change? It
still relies on `wc_str()` returning a pointer to cached value in the
buffer in UTF-8 build, doesn't it?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:6>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Changes (by vadz):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
* status: confirmed => infoneeded_new
Comment:
Sorry if I'm missing something but what does the second patch change? It
still relies on `wc_str()` returning a pointer to cached value in the
buffer in UTF-8 build, doesn't it?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:6>
wxTrac
Jun 1, 2012, 6:43:06 AM6/1/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:7>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Priority: low | Milestone:
* status: infoneeded_new => new
Comment:
In this ticket I am just trying to simplify wxString conversion to
LPCTSTR, LPTSTR and LPARAM in the source code. wc_str() is used in t_str()
in UTF-8 build as before.
I have used wchar_t return type as was discussed:
> implement them as macros
I have missed something?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:7>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Changes (by kosenko):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
* status: infoneeded_new => new
Comment:
In this ticket I am just trying to simplify wxString conversion to
LPCTSTR, LPTSTR and LPARAM in the source code. wc_str() is used in t_str()
in UTF-8 build as before.
I have used wchar_t return type as was discussed:
> we should just change t_str() itself to return either char* or wchar_t*
depending on the build
I have used macros instead of inline functions:
depending on the build
> implement them as macros
I have missed something?
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:7>
wxTrac
Jun 1, 2012, 7:00:22 AM6/1/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:8>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Priority: low | Milestone:
Replying to [comment:7 kosenko]:
could safely return a pointer -- and not a buffer object itself -- then we
could just do it directly from `t_str()` and there was no need for an
extra wrapper function/macro for this.
However we later decided that we couldn't rely on having the buffer
internally in wxString (i.e. it was not safe to return a pointer to it).
This is why we can't make this change.
So conversion to `LPCTSTR` should be implemented as a macro too because we
must have a cast to `TCHAR*` inside it.
I hope I managed to explain myself more clearly this time but please let
me know if you have any questions.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:8>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Comment(by vadz):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Replying to [comment:7 kosenko]:
> I have used wchar_t return type as was discussed:
> > we should just change t_str() itself to return either char* or
wchar_t* depending on the build
But this was prefixed by "If it is safe". I.e. I wanted to say that if we
> > we should just change t_str() itself to return either char* or
wchar_t* depending on the build
could safely return a pointer -- and not a buffer object itself -- then we
could just do it directly from `t_str()` and there was no need for an
extra wrapper function/macro for this.
However we later decided that we couldn't rely on having the buffer
internally in wxString (i.e. it was not safe to return a pointer to it).
This is why we can't make this change.
So conversion to `LPCTSTR` should be implemented as a macro too because we
must have a cast to `TCHAR*` inside it.
I hope I managed to explain myself more clearly this time but please let
me know if you have any questions.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:8>
wxTrac
Jun 1, 2012, 11:27:43 AM6/1/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:9>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Priority: low | Milestone:
I have added LPCTSTR conversion macro and don't change wxString::t_str()
return type.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:9>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
---------------------+------------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: new
Component: wxMSW | Version: 2.9-svn
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
Comment(by kosenko):
Keywords: | Blockedby:
Patch: 1 | Blocking:
---------------------+------------------------------------------------------
I have added LPCTSTR conversion macro and don't change wxString::t_str()
return type.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:9>
wxTrac
Jun 1, 2012, 6:34:15 PM6/1/12
to wx-...@googlegroups.com
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:10>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
----------------------+-----------------------------------------------------
Reporter: kosenko | Owner:
Type: defect | Status: closed
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Resolution: fixed | Keywords:
Blockedby: | Patch: 1
Blocking: |
----------------------+-----------------------------------------------------
Changes (by VZ):
* status: new => closed
* resolution: => fixed
Comment:
(In [71636]) Add wxMSW_CONV_LPCTSTR() and related macros and use them in
wxBase.
Add macros hiding the ugly casts needed to pass wxStrings to Windows API
functions and use them in a couple of places in wxBase to simplify the
code.
Closes #14338.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:10>
#14338: Helpers to convert wxString to LPCTSTR, LPTSTR and LPARAM
Reporter: kosenko | Owner:
Type: defect | Status: closed
Priority: low | Milestone:
Component: wxMSW | Version: 2.9-svn
Resolution: fixed | Keywords:
Blockedby: | Patch: 1
Blocking: |
----------------------+-----------------------------------------------------
Changes (by VZ):
* status: new => closed
* resolution: => fixed
Comment:
(In [71636]) Add wxMSW_CONV_LPCTSTR() and related macros and use them in
wxBase.
Add macros hiding the ugly casts needed to pass wxStrings to Windows API
functions and use them in a couple of places in wxBase to simplify the
code.
Closes #14338.
--
Ticket URL: <http://trac.wxwidgets.org/ticket/14338#comment:10>
Reply all
Reply to author
Forward
0 new messages