C / C++ / MFC

22-Nov-07 23:47

hi,

I am using the function ConvertResourceStringIdToLptstr() to assign the value of the TV_INSERTSTRUCT's pszText for the creation/initialization of a TreeCtrl. (or) to convert the 'resource string' into a pointer to a character string.

<br />
LPTSTR COperDialog::ConvertResourceStringIdToLptstr(UINT nResString)<br />
{<br />
	CString strLoad;<br />
	strLoad.LoadString(nResString);<br />
	LPTSTR lpsz = new TCHAR[strLoad.GetLength()+1];<br />
	CHECK_FOR_NULL(lpsz)<br />
	_tcscpy(lpsz,strLoad);<br />
	return lpsz;<br />
}<br />

And the fuction is used in:

<br />
void COperDialog::AddProjectRootItem()<br />
{<br />
	TV_INSERTSTRUCT     curTreeItem;<br />
<br />
	curTreeItem.hParent =TVI_ROOT ;<br />
	curTreeItem.hInsertAfter = TVI_ROOT ;<br />
	curTreeItem.item.iImage = 0;<br />
	curTreeItem.item.iSelectedImage = 0;<br />
<br />
	curTreeItem.item.pszText = ConvertResourceStringIdToLptstr(IDS_PROJECT);<br />
	curTreeItem.item.cchTextMax=15;<br />
<br />
	curTreeItem.item.mask = TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_TEXT;<br />
	    <br />
	htrProjectRoot = m_Edit->InsertItem(&curTreeItem); <br />
<br />
}<br />

In this situation i am getting memory leaks in ConvertResourceStringIdToLptstr().

So to avoid that where should i insert the delete[]? If i use it inside the function - ConvertResourceStringIdToLptstr() i am not able to get the string displayed!

Kindly give sugesstions!!

Priya Sundar

Nelek22-Nov-07 23:57

22-Nov-07 23:57

You can use the pointer as member or global variable, fill it, use it and when you dont need it anymore... use delete. That way you will have the control about the pointer along the whole class / project.

//in header.h
LPSTR pMyLpstr;
.
//in constructor
pMyLpstr = NULL;
.
//in your code
LPTSTR COperDialog::ConvertResourceStringIdToLptstr(UINT nResString)
{
   CString strLoad;
   strLoad.LoadString(nResString);
   pMyLpstr = new TCHAR[strLoad.GetLength()+1];
  CHECK_FOR_NULL(pMyLpstr)
   _tcscpy(pMyLpstr,strLoad);
   return pMyLpstr;
}
.
//
void COperDialog::AddProjectRootItem()
{
   TV_INSERTSTRUCT curTreeItem;

   curTreeItem.hParent =TVI_ROOT ;
   curTreeItem.hInsertAfter = TVI_ROOT ;
   curTreeItem.item.iImage = 0;
   curTreeItem.item.iSelectedImage = 0;

   curTreeItem.item.pszText = ConvertResourceStringIdToLptstr(IDS_PROJECT);
//once you dont need it anymore
   delete [] pMyLpstr;
   pMyLpstr = NULL;

   curTreeItem.item.cchTextMax=15;

   curTreeItem.item.mask = TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_TEXT;

   htrProjectRoot = m_Edit->InsertItem(&curTreeItem); 
}
.
// To be sure... in the destructor or destroywindow another time
delete [] pMyLpstr;
pMyLpstr = NULL;

Hope it helps

Greetings.

--------
M.D.V. Wink | ;)

If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about?
Help me to understand what I'm saying, and I'll explain it better to you
“The First Rule of Program Optimization: Don't do it. The Second Rule of Program Optimization (for experts only!): Don't do it yet.” - Michael A. Jackson

CPallini23-Nov-07 0:17

23-Nov-07 0:17

That way you have to be very careful.
I think it is an hazardous design.
Probably it would be better if both memory allocation and deallocation are delegated to the caller (e.g. the standard behaviour of Windows API).
Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Priya_Sundar23-Nov-07 0:42

23-Nov-07 0:42

Hi,

CPallini wrote:
Probably it would be better if both memory allocation and deallocation are delegated to the caller (e.g. the standard behaviour of Windows API).

I didnot understand what you are referring to??

Priya Sundar

CPallini23-Nov-07 2:08

Re: Where to put delete[] to prevent memory leaks? [modified]

23-Nov-07 2:08

Was a note on Nelek suggestion.

For instance GetWindowText WIN32 API work that way, the caller has to

(1) Allocate a buffer

const int SIZE = 256;
TCHAR * buffer = new TCHAR[SIZE];

(2) pass it (together as is size) as function argument

if ( buffer)
{
  GetWindowText(hWnd, buffer, SIZE);
}

(3) release the buffer

if (buffer) delete buffer;
buffer = NULL; // invalidate the pointer

BTW all the above stuff can be done (and the life will be easier...) using the stack instead of the heap.
Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Nelek23-Nov-07 2:02

23-Nov-07 2:02

Can you explain a bit what are you referring to? I don't understand you well and I want to learn more.

That this is not the best option... I agree (I know I am just a low/middle skilled programmer that needs to learn more), but... what do you mean with the standard behaviour of windows api? can you put an example?

Maybe something like this?

LPTSTR COperDialog::ConvertResourceStringIdToLptstr(UINT nResString, LPSTR pStr)   //parameter added
{
   CString strLoad;
   strLoad.LoadString(nResString);
   pStr = new TCHAR[strLoad.GetLength()+1];
   CHECK_FOR_NULL(pStr)
   _tcscpy(pStr,strLoad);
   return pStr;
}
//
void COperDialog::AddProjectRootItem()
{
   TV_INSERTSTRUCT curTreeItem;
   curTreeItem.hParent =TVI_ROOT;
   curTreeItem.hInsertAfter = TVI_ROOT;
   curTreeItem.item.iImage = 0;
   curTreeItem.item.iSelectedImage = 0;
;  
   //New Version
   LPSTR pMyLpstr = NULL;   
   curTreeItem.item.pszText = ConvertResourceStringIdToLptstr(IDS_PROJECT, pMyLpstr);
;
   delete [] pMyLpstr;      pMyLpstr = NULL; //As it was
   curTreeItem.item.cchTextMax=15;
   curTreeItem.item.mask = TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_TEXT;
   htrProjectRoot = m_Edit->InsertItem(&curTreeItem);
}

Is this option better than the other one?

-- modified at 8:11 Friday 23rd November, 2007

-- modified at 8:12 Friday 23rd November, 2007

Greetings.

--------
M.D.V. Wink | ;)

CPallini23-Nov-07 2:18

23-Nov-07 2:18

Nelek wrote:
Maybe something like this?

No.

Nelek wrote:
LPTSTR COperDialog::ConvertResourceStringIdToLptstr(UINT nResString, LPSTR pStr) //parameter added

The parameter added is irrelevant.

I meant the standard behaviour of WIN32 API, for instance:

int GetWindowText( HWND hWnd,  LPTSTR lpString, int nMaxCount );

The above function doesn't allocate the buffer, it relies on the one allocated by the caller.

Please note that your suggestion is a perfectly viable solution I only noted that it must be handled with care.

Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Nelek23-Nov-07 4:20

23-Nov-07 4:20

Yes, it can be viable, but I prefer to know the best ways to do things.

CPallini wrote:
int GetWindowText( HWND hWnd, LPTSTR lpString, int nMaxCount );

The above function doesn't allocate the buffer, it relies on the one allocated by the caller.

So before calling the function you have to create the LPTSTR lpString to give him as a parameter, and after the use delete it. And what is the difference with my second option? That the new is inside the function that is being called? For this schema... how can be dealed? To use the new, it must be known the length of the loadString that is calculated inside the called function. (I know, resctructurating and not using the function is a solution)

Greetings.

--------
M.D.V. Wink | ;)

CPallini23-Nov-07 5:28

23-Nov-07 5:28

Nelek wrote:
And what is the difference with my second option? That the new is inside the function that is being called?

Yes, the caller has total responsibility on buffer allocation/deletion. This, IMHO is a quite acceptable design choice.

On the other hand, I insist on saying that yours is a viable solution.

Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Nelek25-Nov-07 21:22

25-Nov-07 21:22

Thanks for answering,

CPallini wrote:
On the other hand, I insist on saying that yours is a viable solution.

I know. But I want to learn, and be better. I like to programm against idiots (although I know is very complicated) and my programms usually are working in 99% of time and situations. But that doesn't mean that are "designly correct". I actually studied electronic engineering not informatic. But that doesn't mean that I may not be able to be a good programer, if I learn with the bests Wink | ;)

Greetings.

--------
M.D.V. Wink | ;)

CPallini25-Nov-07 21:41

25-Nov-07 21:41

Nelek wrote:
I like to programm against idiots

Oh, it is a lost battle. Big Grin | :-D

Nelek wrote:
I actually studied electronic engineering not informatic. But that doesn't mean that I may not be able to be a good programer, if I learn with the bests.

I think the same about me (forced point: I have a physics background Big Grin | :-D

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Priya_Sundar25-Nov-07 22:34

25-Nov-07 22:34

Very glad to know people!

I am also from an ECE(Electronic & Comm Eng). I guess i can get encouraged to do better in programming by you all!!

Thankyou!

Priya Sundar

Priya_Sundar23-Nov-07 0:41

You don't need such a mess

23-Nov-07 0:41

Thanks a lot!!

It did solve my problem for the time being.

As the other person has commented, as this is sort of hazardous, will try if there is any other way of doing it.

Again thanks a lot!

Sometimes, too much into coding... then a simple common sense logic also seems to be complex!

Have a nice evening!

Priya Sundar

CPallini23-Nov-07 0:09

23-Nov-07 0:09

Discard your endless-name-function and simply do:

...
CString strLoad;
strLoad.LoadString(nResString);
curTreeItem.item.pszText =(LPTSTR)(LPCTSTR) strLoad;
...

You can safely do that since you're inserting an item, i.e. you know that your buffer will only read.

Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Priya_Sundar23-Nov-07 0:39

23-Nov-07 0:39

I need to apply this for some 50 strings.. so it will be bulky code if i write these statements for every treectrl item. So i need it as a function which will be just a single line of code!

Thankyou for your suggestion!

Priya Sundar

CPallini23-Nov-07 0:52

23-Nov-07 0:52

Priya_Sundar wrote:
I need to apply this for some 50 strings

Good to know. In such a case you may prefer to allocate a single buffer instead of doing all that allocation/deallocation stuff.
Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Priya_Sundar23-Nov-07 1:03

23-Nov-07 1:03

Could you help me more with respect to the code i have posted? i.e how you want me to allocate single buffer??

Priya Sundar

CPallini23-Nov-07 2:34

23-Nov-07 2:34

At the very beginnning of the process you may allocate a buffer (wide enaough to contain the biggest string you're dealing with), then use it for the retrieval of every string, maybe using the raw WIN32 API

int LoadString( HINSTANCE hInstance, UINT uID, LPTSTR lpBuffer, int nBufferMax);

finally you deallocate the buffer when the last string has been retrieved.

You can also rely on the stack for such purpose:

{
  const int SIZE=1000; // whatever number you need
  TCHAR buffer[SIZE];

  // load string 1
  LoadString( hInstance, uID1, buffer, SIZE);
  // insert item 1
  ...  
 
  // load string 2
  LoadString( hInstance, uID2, buffer, SIZE);
  // insert item 2
  ...
  // load string N
  LoadString( hInstance, uIDN, buffer, SIZE);
  // insert item N
  ...
}

Note: I omitted (leaving it to the reader) the error checking code.
Smile | :)

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.

Priya_Sundar25-Nov-07 22:29