-1

I have a code here. I am using malloc to allocate memory to my struct. One member of this struct is assigned a string using StrDup inside a while loop though other members dont have to change their values. Now, as I am using StrDup, I have to clean memory otherwise there would be memory leaks but the cleaning of memory corrupts my struct malloc. What should I do? Thanks in Advance.

    do
    {
       if( pURL == NULL )
           break ;
       pData->URL = StrDupA(pURL) ;

    }while(pURL != NULL) ;
Abhineet
  • 5,320
  • 1
  • 25
  • 43
  • 1
    C++? `strdup`? `malloc`? This guy needs help, fast! – Xeo Feb 22 '12 at 10:58
  • Your code is extremely confusing. The question you are asking involves a function called `MyThreadProc` which you don't call. The names seems to indicate that the function is executed on a different thread and you also seem to have some synchronization going on but what really is happening is hard to tell. Anyway, you can use two different memory allocation schemes (`malloc` and `LocalAlloc`) but why don't you just stick to just one of these? – Martin Liversage Feb 22 '12 at 11:06
  • 2
    MyThreadProc is called continuously. Please look at the comment below StrDupA function. I know what is going wrong. Wrong is: Malloc of struct and then initializing a struct member with StrDupA and then FreeAlloc the memory. This is somehow interfering with the struct mallocated memory. Can you tell me the proper procedure to free the memory allocated by StrDupA in this case ? – Abhineet Feb 22 '12 at 11:17
  • @AbhineetK7: `StrDupA` allocates memory using `LocalAlloc`. This memory has to be freed using `LocalFree`. On the surface you are using the right calls but how they are performed is a mystery to me. I can only guess because important details are missing but perhaps you are using multiple threads and the same buffer is freed multiple times. Also, if you are asking about help about an error you should explain the error and where it happens. – Martin Liversage Feb 22 '12 at 11:57
  • 2
    @Martin Liversage: If I try to run this program, it shows an Unhandled Exception mostly cause my thread is freeing the pData->pURL. And I dont have any idea how to implement this correctly. So, what I am asking is a secure way to run my code. Any modifications would be most welcome. – Abhineet Feb 22 '12 at 12:35

2 Answers2

1

Well, the simple answer is that you must free pData->URL before replacing it with the result of StrDupA. Like this:

pData->URL = NULL ;
do
{
    pURL = //Some Function Here
    LocalFree(pData->URL) ;
    pData->URL = StrDupA(pURL) ;
}while(pURL != NULL) ;

As for the exception that is being raised, you state in a comment that at some point pURL is NULL. When that happens StrDupA will fail. I can't really advise you on how to fix this because I just cannot get my head around what this code is trying to do.

You are quite possibly also leaking the memory that is created by the function that assigns pURL.

I can't understand why would want to use StrDupA rather than the strdup that the C runtime provides. You are calling StrDupA from Shlwapi.dll. That makes no sense to me. Call the one from the C runtime and free the memory with good old free().

I also don't understand why the loop termination is designed to apparently never terminate. And I've not looked at any of your code other than this single do while loop.

David Heffernan
  • 601,492
  • 42
  • 1,072
  • 1,490
  • 2
    StrDupA is for Ansi. Loop terminates when the function fetching pURL returns NULL. Actually it is a multithreaded code, so the solution you provided will not work. It will throw an Unhandled Exception. Its the same as my code. – Abhineet Feb 22 '12 at 13:23
  • I've added some more thoughts to my answer but I really cannot work out what you are doing. Do note that if `pURL` is ever `NULL` (and it must be for the loop to terminate) then `StrDupA` will fail. Please use the normal `strdup` and not the bogus function from shlwapi.dll. – David Heffernan Feb 22 '12 at 13:39
  • Although I have attempted to answer this question it is essentially impossible to answer comprehensively with any confidence. You really should cut your code down to a minimum before posting and you should be very clear on exactly what errors you are seeing. – David Heffernan Feb 22 '12 at 13:40
  • 2
    There is a check condition before StrDupA to check for pURL == NUll. If true, break from while loop. I have figured out the answer but cant post it as StackOverflow doesnt allows to post within 8 hours of asked question to answer it yourself :-( Anyways thanks for the effort. – Abhineet Feb 22 '12 at 13:50
  • "There is a check condition before StrDupA to check for pURL == NUll." No there is not. Or did you commit the unforgiveable sin of posting code other than that you are using? – David Heffernan Feb 22 '12 at 14:03
  • 2
    My code is too long, so I have omitted the lines that were out of context to this question to give a better readability. Have you not read my question ? My question was how to clear the StrDup allocated memory, so that my struct memory do not corrupts. I have toatal 8 modules interconnected. Do you want me to show them all??? – Abhineet Feb 22 '12 at 14:20
  • Well I already answered your question. But you do need to learn how to ask better questions. – David Heffernan Feb 22 '12 at 14:33
  • 2
    Yeah thanks for that. I will try to frame the question better way next time. – Abhineet Feb 22 '12 at 14:36
0

I have found the solution. Malloc of Struct should be done inside the do-while loop as it will allow each thread to work independently. The only problem now is to initialize pData->hEvent and pData->hSemaphore.

//Before entering do-while loop
hEvent = CreateEvent....;
hSemaphore = CreateSemaphore......;

//inside do-while loop
pData->hEvent = hEvent ;
pData->hSemaphore = hSemaphore ;

//Now in ClearMemory() Function
LocalFree(pData->URL) ;
free(pData) ;

As all the threads have their own struct

Abhineet
  • 5,320
  • 1
  • 25
  • 43