Re: Returning a character buffer from a DLL

Tech-Archive recommends: Repair Windows Errors & Optimize Windows Performance

See below...
On Sun, 3 Jan 2010 07:28:23 -0800 (PST), dushkin <taltene@xxxxxxxxx> wrote:

Hi All,

I wrote a simple MFC application and a simple MFC DLL.
I need to return a string buffer from the DLL in a RunQuery function.

Please see the code below (Note that I cleared some unrelated lines
like catch, etc, which are not related to my problem now to make code
clearer.)

If I check the "TheValue.operator _bstr_t().operator char *()" value
in the DLL ,it gives me the correct value!
But if I check the returned value in the APP, I get junk...

What is wrong here?
****
You are making a fundamental error. See below...
****

What I did was as follows (after many trials, and the code shown here
is not necessarily what I think is the correct one...):

-------------------------
In the App:
-------------------------
char* CSAPI2VPGDlg::GetExtEventID( CString sInternalEventID )
****
I find it odd that you are using the obsolete 'char *' data type here. At least use
LPTSTR so it could be compiled as Unicode! But frankly, I don't see why you would EVER
want to use a string pointer of any type here! A CString would be a good return type, but
char * or even LPTSTR are both Really Bad Ideas, and show an unfortunate throwback to the
poor programming practices of C. There is no reason I could imagine a string pointer as a
reasonable solution to this problem.

I am curious why m_sExtEventId is a char[32]. This seems an exceptionally poor choice of
representation, since it violates good practice (don't use the char data type) and good
practice (don't use fixed-size arrays on the heap or stack). The notion of char should be
considered dead, and the notion of fixed-size character arrays equally dead. They are
used only in VERY rare and exotic circumstances, and I see no evidence that this even
comes close to requiring either poor practice. THINK: AS SOON AS YOU WRITE char * YOU
HAVE MADE A FUNDAMENTAL DESIGN ERROR. It isn't always true, but it is close enough to the
truth to be a good rule to follow. Similarly, AS SOON AS YOU WRITE A FIXED-SIZE ARRAY
INTO WHICH YOU ARE GOING TO WRITE DATA, YOU HAVE MADE A FUNDAMENTAL DESIGN ERROR. Again,
it isn't always true, but it is close enough to consider as a rule to be followed except
in the most rare circumstances, and I do not see any evidence that justifies thinking that
this is such a situation.

Perhaps you have some bizarre notion about "efficiency" here. Forget it. You are wrong.
Efficiency here doesn't matter in the slightest. You are making a database query, then
somehow being concerned about a few tens of nanoseconds? There is more random delay in
the network delays to the server than any badly-formulated concept of "avoiding string
copy" will use up with an extra copy (especially of a short string!) You could copy tens
of thousands of strings in the time it takes the database disk to make one half of a
rotation (this assumes that your query can be resolved in a single disk read)

Is there any correlation between this call of RunQuery, which takes three arguments, and
the DLL version of RunQuery, shown below, which takes only one?
****

{
RunQuery(hRC, m_sExtEventID, sInternalEventID); //m_sExtEventID is a
32 bytes char array member

return m_sExtEventID;
}


-------------------------
In the DLL:
-------------------------

char* CXDBApp::RunQuery(const CString& a_sQuery)
{
//Find field
CString s = a_sQuery;
****
Why is this done? It is unnecessary, and just clutter
****
CString sField = GetFieldForCollect(s);

_variant_t TheValue;
****
This does not need to be declared until much later, so why is its declaration all the way
back here?
****

m_pRecordset.CreateInstance(__uuidof(Recordset));
try
{
m_pRecordset->Open((LPCSTR)a_sQuery,
m_pConnection.GetInterfacePtr(),
adOpenDynamic,
adLockOptimistic,
adCmdText);

while(!m_pRecordset->adoEOF)
{
TheValue = m_pRecordset->GetCollect((char*)_bstr_t(sField));
****
This is just weird.

First, the variable should not be declared until this point.

Second, why are you taking the Unicode value of sField, casting it to pretend it is a
pointer to 8-bit characters, and passing it into a function? This is just WRONG! It
tells the compiler "I know you think this is a Unicode character pointer, but it isn't, so
pretend it is an 8-bit pointer, and use it as such". What is the GetCollect function? (I
don't see it as part of CRecordSet, I looked). Stop thinking char is a useful data type.
****
if(TheValue.vt!=VT_NULL){
m_pRecordset->Close();

//return((char*)_bstr_t(TheValue));
return TheValue.operator _bstr_t().operator char *();
****
At the point where you do this return, you are returning a pointer to a STACK-allocated
variable. When the scope is exited, the _variant_t type is deallocated. This likely
means the string pointed to is deallocated.

In addition, I have no idea why you think returning a pointer to a _bstr_t data type,
which is a Unicode string, and casting this to a char *, could POSSIBLY make sense! And
what are you going to do if the second form, which is more than a little strange anyway,
discovers that there is a Unicode character in the field that has no equivalent in ANSI.

Why are you trapped in a 8-bit character mentality? ASSUME that Unicode is what you
should be doing, and forget that char and char * exist except in very rare and exotic
circumstances, of which this is CLEARLY not an example!

Since you already presume the data type CString exists, you have a couple choices here.

For example, you could declare the return type CStringW (Unicode string), and store it as
CStringW result = _bstr_t(TheValue);
return result;
or, if you are willing to live with the information lossage that comes when you convert
WCHAR to char, you could simply declare a CStringA data type (not a great idea), and do
CStringA result(_bstr_t(TheValue));
return result;
which would also work as long as you don't get a real Unicode character (value > U00FF) in
the data.

In any case, you CANNOT return pointers to local variables or data managed by local
variables from ANY function, independent of whether it is in a DLL or not. The
fundamental error here is assuming you can return a pointer to something that is being
managed by something you have no control over and expect that it will magically be valid.
You should not be surprised that you see garbage on return (and by "garbage" you meant "I
saw some specific values, that looked like this..." and would have given the values, which
were likely something like 0xCDCD, 0xFEEE, 0xDFDF, or something quite similar, which is a
dead giveaway as to what is wrong); I would have been totally amazed if anything
intelligible had been returned at all.

Note that returning a CString is not returning a pointer to a local variable; in fact, it
is returning a CString object, so there is no problem here with scope.

In case you hadn't noticed, this is C++ code. Therefore, there is no need to declare
variables at the top of the context if they are not used until much later. TheValue does
not need to be declared until its first use! So why give it a scope any larger than it
needs?

I think your basic problem is you are still trying to program using the deeply-flawed
models of a long-dead language (C), and doing it in a modern language (C++).
joe
****
}
}
m_pRecordset->Close();
}
}
Joseph M. Newcomer [MVP]
email: newcomer@xxxxxxxxxxxx
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
.

Relevant Pages

  • Re: split a string
    ... But this reads a string into a single char ... Check fgetsfrom the Standard C Library as a way to get string input ... pointer to a pointer to a char. ... To handle arrays remotely. ...
    (alt.comp.lang.learn.c-cpp)
  • Re: copy a string into a 2d array of chars
    ... This split function should allocate a 2D array of chars ... >focus the program the string is not actually split. ... later) is an array of char containing the original contents of the ... The i-th pointer will contain the starting address of the ...
    (comp.lang.c)
  • Re: problem with function
    ... > If you intended copying the string, you need to use the strcpy function. ... I suggest researching hungarian notation for variable names. ... Isn't 'psz' a pointer a an array of characters containing a string? ... pointer to char. ...
    (alt.comp.lang.learn.c-cpp)
  • Re: SBMJOB
    ... command string was being built to pass to a command string interpreter ... As data type *CHAR they are already a "character string" data type ...
    (comp.sys.ibm.as400.misc)
  • Re: type of "string"
    ... Well, the above writing doesn't have anything named "String", so I can't ... The function foo is declared as taking pointer to char. ... That is why compiler gives you a warning. ...
    (comp.lang.c.moderated)