Re: Problem with using char* to return string by reference

<swtbase@gmail.com> ha scritto nel messaggio
news:f04aff05-3fe0-4f92-b7dc-8a5a86f4a884@m45g2000hsb.googlegroups.com...


Consider 'char*' as an obsolete type, in general.
At least, you should use std::string, but better using Unicode UTF-16
strings (i.e. std::wstring, or IMHO better MFC/ATL CStringW class).
You may also consider programming Unicode-aware, i.e. use TCHAR instead of
char, and CString instead of std::string (CString is Unicode aware).

('char' and std::string/CStringA make sense when you want to store Unicode
UTF-8 strings.)


'a' and 'b' are output string parameters, so I would really use CString and
pass it by reference (as Ulrich already suggested):

 void ReadInitRegVal(
   OUT CString & a,
   OUT CString & b,
   OUT float & c,
   OUT float & d )


Here with fixed "BYTE temp[256]" you are assuming that your registry value
data will fit in 256 bytes.
Is this a correct assumption?
It depends on particular data that you are going to query, of course.
You might also want to allocate bigger buffer, e.g. on the heap, using
std::vector:

  // #include <vector>
  std::vector< BYTE > temp( <number of bytes> );

In this case, the address of the first byte in the buffer is &(temp[0]).

Moreover, note that you can ask RegQueryValueEx the exact amount of data
required for a specified value name.

You can read details for that in MSDN documentation of that API:

http://msdn.microsoft.com/en-us/library/ms724911.aspx


You should code in a Unicode-aware way: decorate your string literals using
_T(""):

  RegQueryValueEx(
    hKey,
    _T("Data0"), // <--- _T("")
   NULL,
   NULL,
   temp,
   &DataSize );

If you find boring to manually decorate all your string literals using
_T(""), you might consider a little simple macro I developed for VS:

 http://code.msdn.microsoft.com/UnicodeTDecorator

with it, you can type a string literal, e.g. "Data0" and press a short-cut
key on the keyboard, and the string got automatically decorated by the
macro.

  "Data0"<cursor>

  <type key to call the macro>

  _T("Data0")<cursor>

Moreover, the RegQueryValueEx returns an error code. To build robust quality
code, you should check the return value, at least in debug builds:

 LONG result = RegQueryValueEx( ... );
 ATLASSERT( result == ERROR_SUCCESS );

or better you may consider returning error codes to the caller, or throwing
C++ exceptions on errors... (it depends on your particular problem and
design).


Forget about strcpy and old C string functions.

You should use more modern and robust functions from SafeString library, for
example:

 StringCbCopy
 http://msdn.microsoft.com/en-us/library/ms860399.aspx

Your code may become:

  StringCbCopy(
    <destination pointer>,
    sizeof( <destination buffer> )
    reinterpret_cast<LPCTSTR>( temp ) // source
  );

However, if you use CString as suggested instead of "char * a", and
considering that you have the string temporarily stored in 'temp' buffer (I
assume that this string is null-terminated with \0), you can write:

  // 'CString & a' is a function parameter

  // a = temp
  a = CString( reinterpret_cast< LPCTSTR >( temp ) );

This applies also for 'b'.


See what I wrote for 'a' (e.g. decorate "Data1" using _T("Data1") , etc.)


Code Unicode-aware, so don't use atof, but _ttof.


As above.


Use just:

  CString a;
  CString b;

instead of raw C-style char buffers.

HTH,
Giovanni