You will leak memory every time getFile() is called One solution would be: while(i==1)//forever Loop { ... char* temp = getFile( request ); strcpy(response,temp); free( temp ); ... }.
You will leak memory every time getFile() is called. One solution would be: while(i==1)//forever Loop { ... char* temp = getFile( request ); strcpy(response,temp); free( temp ); ... }.
– Peer Stritzinger Feb 15 at 13:31 2 We don't know - that part of the source code isn't provided. I assumed it was taken care of somewhere else. – Nick Feb 15 at 13:34 But if response already points to valid memory it should be just passed to getFile() and nothing should be malloced at all – Peer Stritzinger Feb 15 at 13:37 1 Yep, I totally agree.
Hence +1 for your answer :) – Nick Feb 15 at 13:38 Who cares where response gets it's memory. There is not enough information in the original post to answer that question. But the code as posted leaks memory like a sive and nick's response fixes that memory leak.
So nick's answer is spot and gets my +1. – mrsheen Feb 15 at 13:38.
Then you can free() the memory in main(). I'm not certain, but don't see why that would not work.
Response" is stored for late use in main or be used in strtok(). It depends at the state of the program. That's why strcpy used at that situation.
– matafleur Feb 15 at 14:40.
Strcpy(response,getFile(request)) is likely to crash your program because response points to read-only memory.
2 Actually response isn't declared in the provided source so we don't know if its read-only or not. – Nick Feb 15 at 13:32 1 @Nick - My take too: it was probably among the parts that were redacted. Going strictly on what's included in the question, the code cannot crash because it won't even compile, even if the extra periods are removed.
– Michael Kjörling Feb 15 at 13:35 1 @Nick, I misread response for request. – lhf Feb 15 at 13:59.
Strcpy(response,getFile(request)); Should be splited into: char *tmp = getFile(request); strcpy(response, tmp ); free( tmp ); This is sometimes refered as strdup() style allocation which is not recommended. You should allocate the buffer in the calling code like: char *buf2 = malloc(strlen(buffer)); and call getFile( buf2, strlen( buffer ), input ); // use buf2 free( buf2 ).
In embedded systems malloc() is often used to allocate memory on startup that will be used while the system is running and never free()d. But you shouldn't do something like this periodically since it will use up your memory if you don't free() it again. What you are doing in your example is leak one buffer worth of memory each round in the while loop.
What you should do is either, call free() in your loop but be aware this can still fragment menmory which is especially bad in an long running embedded system. If you know in advance how much memory you will need in advance: malloc the buffer once outside the loop and reuse it in the loop: response = malloc(MAX_BUF_SIZE); while (1) { get_file(response, MAX_BUF_SIZE); /* always good to pass the buf size and check */ ... use response ... } Or if you don't know the size in advance one pattern can be: response = NULL; size = 0; while (1) { get_file(&response, &size); ... use response ... } void get_file(char **buf, int *s) { size = ... somehow determine the needed size ... if (size > *s) *buf = realloc(*buf, size); /* only does free/malloc internaly if it has to no room */ strncpy(*buf, whatver, size); } And you should always use strncpy and never strcpyplease! The use of strcpy in your example is confusing, it can't be seen where response gets its memory, is it statically allocated?
Has it been malloced before? If response already points to a vaid buffer you should just pass the pointer to the buffer to your getFile() and directly copy the buffer there. If response doesn't point to a valid memory buffer this will not work anyway.
Whenever you're creating an interface to some functionality you need to think about resource management - what's the lifetime of various objects and how the lifetime of those objects is managed. This is particularly true for languages like C and C++ which don't implement garbage collection, but even in garbage collected languages, some thought needs to be given to this (for non-memory resources and maybe to ensure that object references aren't held indefinitely). Anyway, for C, APIs can deal with 'output' objects in several ways.
Some of the more common patterns are: have the caller provide a buffer or object to place results in have the callee allocate a buffer or object, and return a pointer to the allocated object have the caller return the object directly from the function - this only works for items that have a fixed size (an intrinsic type or a struct) - it doesn't work for arbitrary strings. So I won't discuss this further. For option 1, the caller provides a buffer to place results in by passing a pointer.
For variable length data, like a string, it's important that the interface also allow the caller to pass in the size of the output buffer so the function can avoid writing outside of the buffer. For your getFile() function, the prototype might look like: int getFile(char const* request, char* result, int result_size); The caller passes in a pointer to place the results and the size of the passed in buffer. The function might return a negative number on failure (say a network failure) and the size of the result on success.
If the size returned on success is larger than the buffer provided, then the caller knows that the full result wasn't placed in the buffer (since it's not large enough). If you go this route, it's important to take into consideration the '\0' terminator character and clearly indicate whether or not it's included in the return value and whether the buffer is terminated if the result is too large (which I recommend). Also, make sure that if a zero-sized buffer is passed in, the function doesn't write anything to the buffer whether or not a non-NULL pointer is passed in (it's a common pattern to have the function return the required size of the buffer in this case).
For option 2, the function will allocate an appropriate buffer and return a pointer to that to the caller. In this case, there needs to be a way for the caller to free the buffer when it's done with it to avoid a memory leak. One way is to document that the caller needs to call free() on the pointer returned by the function (implying that the function will allocate the pointer using malloc() or calloc()).
Another is to have the interface for using the function also include a deallocation routine, maybe getFile_free() (I'd definitely rethink the naming of these functions if I were to go this route). This gives the implementation of this API the freedom to allocate the buffers it returns however it sees fit - it's not constrained to use malloc()/free() (though it could). For an embedded system (especially small ones - maybe not Linux-based systems), I think that people might opt for option 1.
That gives the user of the API the freedom to avoid dynamic memory allocation altogether, and it's common for embedded systems to not use dynamic memory. Also, a system using dynamic memory allocation can still use option 1's pattern - it just takes a bit more work, but that work can be wrapped in something that looks exactly like option 2 anyway, so you can have your cake and eat it too.
Oh. Thank you for this explanation. Not newbie in C but not experienced well.
– matafleur Feb 15 at 19:41.
Yes it will cause memory starvation eventually, which will lead to the crashing of your program. A simple rule that helps solve this kind of problem is to always remember to free memory in the same scope as where it has been allocated, if possible. In your case, it is obviously impossible to free it after the call to return, therefore it makes sense to actually allocate it in the parent scope, ie.In the main function, passing the allocated pointer as a second argument to the getFile function, and directly write into it, assuming you make sure it is large enough to contain all characters.
Firstly, the *retVal is placed in the getFile function, thus the scope of *retVal is limited to that particular function and the memory space it uses will automatically be set to FREE once the function terminates/returns. The scope of a varible decides its lifetime/usability, also once the lifetime is over and you don't free the block the data stays there, it can be accessed through pointer, however the memory block is kind of MARKED AS FREE and is overwritten afterwards without any troubles. Secondly, Nick is very correct,you will waste memory every time getFile() is called, as it is in main function and has scope till the program runs.
Hope this helps, if it doesn't I'll be glad to help just let me know :).
Thank you for the explanation. (: – matafleur Feb 15 at 14:53.
I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.