Link to home
Start Free TrialLog in
Avatar of didou
didou

asked on

Returning by reference

I have the following function:

int NL_EnumVars(
      IN HSESSION hSessionA,
      OUT const char
             *lpszVariableName,
      OUT const char
             *lpszVariableValue,
      OUT int
             *RestartFlag)

Which I am calling with the following
code:

      const char
lpszVariableNameB=NULL;
      const char  lpszVariableValueB=NULL;

      int RestartFlagB=NULL;

        const char VarName=NULL;

      NL_EnumVars(hSessionA, &lpszVariableNameB, &lpszVariableValueB, &RestartFlagB);

When I do:

      strcpy(VarName, &lpszVariableNameB);

It crashes on me.  How do I get it to
return the "OUT" values as shown in
the definition?

THanks,
Didou
Avatar of dhymes
dhymes

Mmm, you have to pass in a null terminated value to strcpy. You are passing in the address of a single character, so strcpy does not know where to terminate.
Avatar of didou

ASKER

How would I pass a null terminated value to strcpy?  
THanks,
Didou
dhymes is right, but there are other problems too.

For example in

const char  *lpszVariableNameB=NULL;
strcpy(VarName, &lpszVariableNameB);

the pointer lpzVaraibleNameB doesn't point to anything.  it must point to a character array that can be used to store the string you are copying.  Like

const char *lpszVariableNameB = new char[100];

which makes the pointer point to an array of 100 characters.

Now this is not necessarily the way to fix the problem, it is a possible solution, but there are likely to be better ones.  

Unfortunately, it is not really easy to understand what you are trying to do.
>> How would I pass a null terminated
>> value to strcpy?  

Well you need two things.  You need an array containing the string to be copied and an array to copy it to.  howe you get those really depends on the circumstances, and these are very unclear.  But for example, to copy a constant string to a variable you might do

const char *ConstString = "some string";
char VarString[20];

strcpy(ConstString,VarString);
//Also NL_EnumVars is expecting pointers to strings, you are not passing in pointers to strings, you are passing in a pointers to a single characters.
=================================
try the following.

const char lpszVariableNameB[SOMESIZE];
const char lpszVariableValueB[SOMESIZE];
const char VarName[SOMESIZE];

memset(VarName,NULL,SOMESIZE);
memset(lpszVariableNameB,NULL,sizeof(lpszVariableNameB));
memset(lpszVariableValueB,NULL,sizeof(lpszVariableValueB));


NL_EnumVars(hSessionA, lpszVariableNameB, lpszVariableValueB, &RestartFlagB);

strcpy(VarName, lpszVariableNameB);
Now it seems like the NL_EnumVars() function might be trying to return information to you usng those character pointer parameters.  That would mean that to use this function you would have to pass the function pointers to character arrays that you have allocated and want filled with information by the function, like

char lpszVariableNameB[100]; // big enough?
char  lpszVariableValueB[100];
int RestartFlagB=0;
NL_EnumVars(lpszVariableNameB,lpszVariableValueB,&RestartFlag);

See how the code has allocated space for two character arrays and an integer and passes pointers to these 3 things to the function.
the code you ar showing to exhibit your example just aint right.

>int NL_EnumVars(
>IN HSESSION hSessionA,
>OUT const char *lpszVariableName,
>OUT const char *lpszVariableValue,
>OUT int *RestartFlag)

you've said these are OUT params, yet they are const.  So you cannot change them.

>const char lpszVariableNameB=NULL;
>const char lpszVariableValueB=NULL;

aren't these supposed to be STRINGS .. you've just declared tham as single characters with ASCII NUL value.

>int RestartFlagB=NULL;

style: better to use 0 for ints

>const char VarName=NULL;

same here .. should be a string

>NL_EnumVars(hSessionA, &lpszVariableNameB,
>&lpszVariableValueB, &RestartFlagB);

This will not modify the variable names and values .. the function is declared as const.  And even if the fuction delcaration were changed, the cars are declared const.

>When I do:
>strcpy(VarName, &lpszVariableNameB);

This shouldn't even compile, because VarName is a char and not a char*, and if it were, it is const anyway.

Also none of these things that should be strings actaully point to any storage.  Either your calling program should do something like

const char lpszVariableNameB[40];

or your called function should take a char** plpszVariableName
and allocate some storage and return a pointer to the caller (who would then need to deallocate).

you should either also give VarName some storage (eg char VarName[30]) or you should use VarName = strdup(...) to return a pointer to some allocated storage (that you will later have to free).

Suggestion: get the delcarations right and allocate some storage for the strings.

for example, to copy a constant string to a variable you might do

const char *ConstString = "some string";
char VarString[20];

strcpy(ConstString,VarString);

-----------------------------------

I think you have it backward.  It should be:

strcpy(VarString, ConstString);


The function prototype of 'strcpy' is:

char* strcpy(char* s1, const char* s2)


This copies the string 's2' into the character array 's1'.  The value of s1 is returned.
>> I think you have it backward.  It should be:
YES!  Opps.  I haven't used strcpy() in a "real" program in 10+ years.  Still no excuss.
RONSLOW,

nietod and I are both working with didou, don't you think you should wait for us to get a response before proposing an answer, I think that is the way it is supposed to work, otherwise i would have proposed my code as an answer.

To many problems with this site!
Dave
>> To many problems with this site!
Good days, bad days, comes sun, comes rain. I'm convinced there were no bad intentions.
"dhymes" has a point!!

I've had the same thing done to me a few times, but because I don't really care about the points, it didn't matter to me.

It's all a matter of courtesy and respect.
Avatar of didou

ASKER

Thanks for all the comments, but I am
still struggling:

I've changed my code to the following:

      char lpszVariableNameB[100];
      char lpszVariableValueB[100];
      char VarName[100]="";
      int RestartFlagB=0;

      NL_EnumVars(nSessionIndexA, lpszVariableNameB, lpszVariableValueB, &RestartFlagB);

and redefined NL_EnumVars to:

int NL_EnumVars(
      IN HSESSION hSessionA,
      OUT  char *lpszVariableName,
      OUT  char *lpszVariableValue,
      OUT int *RestartFlag)

But still when I get out of the call
to NL_EnumVars, the values of
lpszVariableNameB and lpszVariableValueB
are empty.

Help!!!

Thanks,
Didou
KangaRoo,

Ya, this is a great site and your right, good days, bad days!

Regards,
Dave


didou,

you will have to post what your NL_EnumVars function is doing, post all of your code and I will take a look at it.

Regards,
Dave
Avatar of didou

ASKER

Dave:

Actually, just this simple code doesn't
work!

int NL_EnumVars(
IN HSESSION hSessionA,
OUT  char *lpszVariableName,
OUT  char *lpszVariableValue,
OUT int *RestartFlag)
{
lpszVariableName = "var1";
lpszVariableValue = "2";
return 1;
}
>> Actually, just this simple
>> code doesn't
>> work!
No, I suspect it works fine.  it just doesn't do what you thought it would.

Inside the function NL_EnumVars() it makes the character pointer called lpszVaraibleName point to a string that contains "var1".

That DOES work.  However, this changes ONLY the value of the pointer INSIDE the function.  the caller's copy of the pointer is not affected so in

char *AStringPtr = NULL

NL_EnumanVars(AStringPtr....);

the varialbe AStringPtr remains unchanged.  

If you made the pointer parameter a reference parameter like

int NL_EnumVars(
IN HSESSION hSessionA,
OUT  char *&lpszVariableName,
OUT  char *&pszVariableValue,
OUT int *&RestartFlag)

then the caller's copy of the pointers could all be changed by the function.

However, it is very UNLIKELY that this what you want.  (We can't say for sure...)

If you would describe to use what the function needs to do, then we could probably help you.  (I've suggested this before.)


===========================
NL_EnumVArs

const char lpszVariableNameB[SOMESIZE];
const char lpszVariableValueB[SOMESIZE];
const char VarName[SOMESIZE];

memset(VarName,NULL,sizeof(VarName));
memset(lpszVariableNameB,NULL,sizeof(lpszVariableNameB));
memset(lpszVariableValueB,NULL,sizeof(lpszVariableValueB));

=============================
int NL_EnumVars(
IN HSESSION hSessionA,
OUT  char *lpszVariableName,
OUT  char *lpszVariableValue,
OUT int *RestartFlag)
{
//lpszVariableName = "var1";
memcpy(lpszVariableName,"var1",4);
lpszVariableValue = "2";
memcpy(lpszVariableName,"2",1);
return 1;
}

This will work, but why would you want to pass a ref to a const char back? This does not make any since. Not really sure what you are trying to accomplish.


Oh, and please reject RONSLOW's answer since he went out of turn and did not follow expert exchanges ules/suggestion on how to post an answer. An answer normaly should be invited by the person who posted the question once all discussion has been completed.

Regards,
Dave
Avatar of didou

ASKER

Actually here is the full code.
I believe the problem is at malloc
statements.  When I took them out
it all seems to be working fine.
Question: NL_EnumVars resides within
a DLL.  When the call is made to
NL_EnumVars, what happens to the
memory?  Who should clean it up?

---------------------------------------

int NL_EnumVars(
      IN HSESSION hSessionA,
      OUT  char *lpszVariableName,
      OUT  char *lpszVariableValue,
      IN int RestartFlag)

{
      int nSessionIndex;

      // If the session handle is invalid,
      if ((getSessionIndexHS(hSessionA,&nSessionIndex,FALSE)) != GI_FOUNDOK)
      {
            // Return a complaint.
            return(-INVALIDHANDLE);
      }

      s_nTotalVariables =      g_SessionTable[nSessionIndex].nNumVariables;

      if (RestartFlag ==1) {
            s_nVariableEnumIndex = 0;
      }

      //all variables have been returned
      if (s_nVariableEnumIndex >= s_nTotalVariables) {
                  s_nVariableEnumIndex = 0;
                  return(-RANOUTOFVARIABLES);
      }
      //lpszVariableName = ( char *) malloc(sizeof(g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex]));
      //lpszVariableValue = ( char *) malloc(sizeof(g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]));

      strcpy(lpszVariableName,g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex]);
      strcpy(lpszVariableValue, g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]);

      s_nVariableEnumIndex++;
      return (1);


}
Yes it would have helped if you would have shown us this when you first posted your question. There should be no problem with performing your malloc to dynamically allocate the storage you need.

In your malloc statement you need to allocate one more character and insure that the string being passed to strcpy is null terminated, you can insert the null your self.

The unfortunate thing about the malloc is that the calling routine is going to have to know to clean up the memory when complete. You should always attempt to force the caller of your routine to allocate the memory, this way he knows he has to clean it up.

Regards,
Dave
>> why would you want to pass a ref to a const char back?
A reference to a constant character pointer makes sense.  The function can change the pointer to refer to differen constant characters....

>> NL_EnumVars resides within
>> a DLL.  When the call is made to
>> NL_EnumVars, what happens to the
>> memory?  Who should clean it up?
You have to tell us who should clean it up.  We STILL don't know what the objective is!!!!  You need to tell us what you want to accomplish.  Posting bad code doesn't help because it is bad code.  

If the DLL will allocate memory and the EXE will delete it, then you must use the DLL version of the RTL (run-time library) for both the DLL and the EXE   This is an option in the project settings.

There are numerious problems:

You should be using new, not malloc() to allocate memory.  There is basically no reason to use malloc() ever in C++.

In
>> malloc(sizeof(g_SessionTable[nSessionIndex].
>> paszVariableNames[s_nVariableEnumIndex]));
it is hard to say what the value is that you are passing to malloc()   But I strongly suspect it is the wrong value.  What is right?  probably something using strlen().  But once again, NO ONE can tell you because we don't know what you are trying to accomplish.

If the length pased to malloc() is wrong, the strcpy() may crash.  

The function changes the local values of the two character pointers passed to it.  As I said in my previous comment that has no afffect on the caller's values.  i.e the caller does not get this information!!  i.e this design cannot return information to the caller.  Do you want it do?

Now the function allocates memory and never deletes it, it does not pass the value back to the caller--though I bet you want to, so the caller can never delete the memory.  so you have a permenant memory leak.

If  you want help, don't post code.  Post an explanation, a clear one, of what you want to do.
int NL_EnumVars(
IN HSESSION hSessionA,
OUT char *lpszVariableName,
OUT char *lpszVariableValue,
IN int RestartFlag)

{
int nSessionIndex;

// If the session handle is invalid,
if ((getSessionIndexHS(hSessionA,&nSessionIndex,FALSE)) != GI_FOUNDOK)
{
// Return a complaint.
return(-INVALIDHANDLE);
}

s_nTotalVariables = g_SessionTable[nSessionIndex].nNumVariables;

if (RestartFlag ==1) {
s_nVariableEnumIndex = 0;
}

//all variables have been returned
if (s_nVariableEnumIndex >= s_nTotalVariables) {
s_nVariableEnumIndex = 0;
return(-RANOUTOFVARIABLES);
}
lpszVariableName = ( char *) malloc(sizeof(g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex]));
lpszVariableValue = ( char *) malloc(sizeof(g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]));

// I would say to get rid of strcpy and avoid using it all together. Use memcpy, it does not care about nulls, but requires you to provide the size.
// Insure that lpszVariableName and lpszVariableValue are large enough to hold the value being copied in.
memcpy(lpszVariableName,g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex],sizeof(g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex]);
memcpy(lpszVariableValue, g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]);

s_nVariableEnumIndex++;
return (1);


}
Correction

int NL_EnumVars(
IN HSESSION hSessionA,
OUT char *lpszVariableName,
OUT char *lpszVariableValue,
IN int RestartFlag)

{
int nSessionIndex;

// If the session handle is invalid,
if ((getSessionIndexHS(hSessionA,&nSessionIndex,FALSE)) != GI_FOUNDOK)
{
// Return a complaint.
return(-INVALIDHANDLE);
}

s_nTotalVariables = g_SessionTable[nSessionIndex].nNumVariables;

if (RestartFlag ==1) {
s_nVariableEnumIndex = 0;
}

//all variables have been returned
if (s_nVariableEnumIndex >= s_nTotalVariables) {
s_nVariableEnumIndex = 0;
return(-RANOUTOFVARIABLES);
}
lpszVariableName = ( char *) malloc(sizeof(g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex]));
lpszVariableValue = ( char *) malloc(sizeof(g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]));

// I would say to get rid of strcpy and avoid using it all together. Use memcpy, it does not care about nulls, but requires you to provide the size.
// Insure that lpszVariableName and lpszVariableValue are large enough to hold the value being copied in.
memcpy(lpszVariableName,g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex],sizeof(g_SessionTable[nSessionIndex].paszVariableNames[s_nVariableEnumIndex]);
memcpy(lpszVariableValue, g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]),sizeof(g_SessionTable[nSessionIndex].paszVariableValues[s_nVariableEnumIndex]);

s_nVariableEnumIndex++;
return (1);


}

Avatar of didou

ASKER

nietod, dave, all:

Sorry about the cripticness of my posts.
Basically the function merely returns
a variable name and variable value
from an array, starting from the first
item down the line, one at a time for
every call.  If it runs out of values
it returns value -RANOUTOFVARIABLES
and points again to the top of the
list (I use g_SessionTable[nSessionIndex].paszVariableNames and
g_SessionTable[nSessionIndex].paszVariableValues).

the function resides within a non-selfregistering dll.

Hope this clears up the context.

thanks/didou
Okay, The C way (which is what you are closest to here) would be to have the caller allocate space for the strig to be returned and pass a pointer to that space, then the procedure fills in the space.  A simple example would be

void Function(char *BufPtr)
{
   strcpy(BufPtr,"somestring");
}

   *   *   *

char Buffer[100];

Function(Buffer); // This fills Buffer with "somestring".

Note how the function does not change the pointer that is passed to it.  It never changes BufPtr.  That is because BufPtr is already set.  It is set to the place where the caller wants the data to be returned.  So the function just uses the pointer to place information where the caller wants it.  

Does that make sense?

Now there are problems with this C-style approach.  If the caller doesn't allocate enough space, like if the caller did

char Buffer[2];

the string would overflow the allocated buffer and cause a crash.  These sort of errors can be very unpredicatable too, they may not be caught during debugging and may show up when the program is being used.  

That is one reason why C++ is so much better than C.  C++ provides features to let us avoid these problems.  For example, you can use a string class to return the information.  The string will expand to whatever length is needed to store the text.  In addition the string class will "behave" more like a simplee data type.  it can be copied using the = operator instead of strcpy() for example.

continues
ASKER CERTIFIED SOLUTION
Avatar of nietod
nietod

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
nietod,

I agree but will put up a warning flag here.

Using the strcpy function is no recomended when the size of "somestring" is not known by the caller. Always attempt to force the caller of your function to pass in the size of the string (WHY Not, the caller allocated the memory!, so knows the size of the string). Try and use memcpy when you don't know what "somestring" will contain.

>>void Function(char *BufPtr)
>>{
>>   strcpy(BufPtr,"somestring");
>>}

void Function(char *BufPtr, int iBufSize)
{
   memcpy(BufPtr,"somestring",iBufSize);
}


>> Using the strcpy function is no recomended
>> when the size of "somestring" is not known
>> by the caller.
That is a major point I was trying to make and one reason I suggested the use of a string class.

>> Try and use memcpy when you don't know
>> what "somestring" will contain
That doesn't seem to help, at least not in this case.
nietod,

Yep, no doubt about the fact that using a string class would solve this problem. Thank god for OO and C++, they have taken this type of problem out of scope for most of us. But once in a while we all have to dig in and work with the old C functions.

>> Try and use memcpy when you don't know
>> what "somestring" will contain
That doesn't seem to help, at least not in this case.

Ya, I always push the use of memcpy. The good thing about memcpy is that it does not care about what the string contains, so it could contain binary data in it and it would not care. Not to say anything bad about a string class but, one problem with string classes is that you can not store NULL chars, so string classes don't do much good when dealing with binary data (the MFC CString for instance). But, anytime you can use a string class you should (if you are developing using C++).

Regards,
Dave
>> good thing about memcpy is that it does not
>> care about what the string contains, so it could
>> contain binary data in it and it would not care
True, but then the code that uses the string does care.  i.e if you don't have a NUL terminating the string, then you can use memcpy to copy it, but then the code that uses the copy will porobably crash...

>> one problem with string classes is that you can not store NULL chars
It depends on the class, but many do.  I certainly support any value in my class.  The MFC CString class does and so does the STL string class.  (You said that CString doesn't support NUL characters, but that is not correct.)
Yes, you can store null chars in a CString class, but normally string classes use the null char to control the length attribute, so once you store a null then you become responsible for maintaining things like the string length and so on (Pain in the rear if you ask me). So, for binary data, although you can, I would not.
The majority of string classes don't do that.  They maintain string lengths and thus NUL characters within string don't cause problems.  The only time NUL is treated specially is when you initalize the string with a C-style string.

(I store binary files in strings--no problem.)
> The majority of string classes don't do that.

Lost me, the majority of string classes don't use '\0' to determine the length attribute for the string!

Well, Rogue wave, RWCSTring, and MFC CString both do (Ya bad thing maybe). But you need some character to delimit your string for the length attribute.

If i did,
CString csMyString("abcd\0rrrrrrrr");
my length would be 4, unless I updated the length attribute myself.
>> But you need some character
>> to delimit your string for the length attribute.
Not if you store the string length and a data member and then don't interpret stored NUL characters specially, this is relatively common.

>> CString csMyString("abcd\0rrrrrrrr");
As I said, the class will treat a C-style string as being NUL terminated--It has no other choice. So it can't tell that the initial string has those additional characters.  But that is a C-style string, not the string class.  The strinf class can support internal NUL characters.  Like if you do

CString S1 = "ABC";
char *ChrPtr = S1.LockBuffer();

*(Chr+1) = 0;
S1.UnlockBuffer(3);

CString S2 = S1;

Now both S2 and S1 are 3 characters long and have a NUL character in the 2nd position.   (This is not a "trick".  It is legal.)
Ok, so anyway back to didou question. Did you get the answer you were looking for?
RONSLOW changed the proposed answer to a comment
Avatar of didou

ASKER

Thanks all for your wonderful input.