3

Suppose I have the following piece of code in my program:

char *ptr;
ptr=malloc(sizeof(char)*10);
ptr=malloc(sizeof(char)*10);
ptr=malloc(sizeof(char)*10);
ptr=malloc(sizeof(char)*10);

Will a pointer to the same memory block be assigned to ptr each time,or a separate piece of memory be reserved each time and its pointer assigned to ptr,resulting in memory leak each time malloc() is called?

I am still learning C so bear with me if it's too basic.I tried googling but found no answer.

EDIT::

Thanks for your answers.Please tell me if this approach of me deals with the memory leak risk.My program simply asks for names of 5 people and displays it,without using static arrays.After reading your answers,I put the free(ptr) inside the loop,else before I had planned to use it only once outside the loop,after the loop.Am I correct now?

#include <stdio.h>
#include<stdlib.h>
#include<string.h>

int main ()
{
char *names[5],*ptr;
int i;

for(i=0;i<=4;i++)
{
    ptr=malloc(sizeof(char)*10);
    printf("Enter name no.%d : \n",i+1);
    scanf("%s",ptr);
    names[i]=malloc(strlen(ptr)+1);
    strcpy(names[i],ptr);
    free(ptr);


}
for(i=0;i<=4;i++)
printf("%s\n",names[i]);

}
Thokchom
  • 1,602
  • 3
  • 17
  • 32
  • short answer memory leak – qwr May 13 '13 at 14:52
  • @huseyintugrulbuyukisik I didn't understand what you said.Can you make it a little clear? – Thokchom May 13 '13 at 14:56
  • 3
    @Thokchom He's rambling about details which don't matter for your question (and likely your next 50 questions). –  May 13 '13 at 14:57
  • Please check my edit and tell me if my approach to deal with memory leak using `free()` is correct.I had planned to **free** `ptr` outside the loop. – Thokchom May 13 '13 at 15:00
  • 1
    It's commonly pointed out in cases like these that `sizeof (char)` is always going to be 1, so you can just as well remove it. It adds nothing but clutter. – unwind May 13 '13 at 15:21
  • @Thokchom The ptr = malloc() should be outside the loop as it should only allocate a memory segment once. You can have it inside the loop but then you must `free(ptr)` at every lap in the loop. But please note that it doesn't make any sense to even use dynamic memory allocation for `ptr` in this case. – Lundin May 13 '13 at 15:21
  • @Lundin Well,using dynamic memory for `ptr` does seem counter-intuitive in this case,but I was asked to write this program to overcome a drawback of using a static array--that every name has to be of fixed size,resulting in unused bytes when names are smaller than the memory allocated to them.Allocating memory dynamically based on the input (using an intermediate buffer) seems to solve the problem.Can you please suggest a better approach?How else I can do it? – Thokchom May 14 '13 at 01:18
  • You are allocating `ptr` to a constant size, so it has the very same drawbacks that a static array has, plus it is far slower and more bug prone. `names` should be allocated dynamically, but the `ptr` variable should be allocated statically, with the size of the longest name allowed (worst case). – Lundin May 14 '13 at 06:13

7 Answers7

6

malloc() will never return the same memory block multiple times, unless (of course) it has been free()'d since the last time it was returned. This is guaranteed by the C standard. Therefore, your code also leaks memory. Any memory allocator that handed out the same piece of memory twice would be worse than useless: Applications would step on their own toes, using the same piece of memory for different purposes, likely concurrently.

Edit: Barring buffer overflow issues, your code is correct in that it frees the 10-char buffers referenced via ptr. Calling free(ptr) just once, outside the loop, would indeed be incorrect. However, your code (as shown here) does not free the memory allocated later in the loop body stored in names[i].

  • Actually, after `free(p); p = malloc(n)` the pointer `p` may be unchanged. – Fred Foo May 13 '13 at 14:53
  • @larsmans True, that wording was misleading. I Edited, hopefully it's both correct and comprehensible. –  May 13 '13 at 14:55
  • @larsmans In `free(p)` we free the memory pointed by `p`.Why then the pointer `p` may be unchanged if we use `p=malloc(n)` thereafter?Isn't it guaranteed to point to the memory block allocated by the new call to `malloc()`?I am confused about it.please clarify. – Thokchom May 13 '13 at 15:03
  • @delnan Please see my edit and tell if I am correct there in my program's approach to deal with leak. – Thokchom May 13 '13 at 15:04
  • 1
    @Thokchom: since the memory block pointed to by `p` is `free`'d, it can be reused by subsequent calls to `malloc`; that's the whole point of `free`. I.e. you *can* get the exact same pointer back. – Fred Foo May 13 '13 at 15:06
  • @larsmans Same pointer all right,but no memory leak risk at all eh? – Thokchom May 13 '13 at 15:07
  • @larsmans I had planned to use a `free(ptr)` only once after the loop,but after reading the answers,I put it in the loop.I did the right thing eh?No memory leak now? – Thokchom May 13 '13 at 15:08
2

Yes, each malloc call will allocate a new block, which means that the previous one will be leaked.

EDIT: answer to the edited part of the question

You are now correctly releasing the blocks to which ptr points, however you are not releasing those allocated for names. You would also need:

for(i=0;i<=4;i++) {
   printf("%s\n",names[i]);
   free(names[i]);
}

In your case, you could skip using ptr and work on names directly. Here's an equally [un]safe version:

char *names[5];
int i;

for(i=0; i < 5; i++)
{
    names[i] = malloc(10);
    printf("Enter name no.%d : \n",i+1);
    scanf("%s",names[i]); // what if name is longer than 10 characters?
}

for(i=0; i < 5; i++)
{
   printf("%s\n",names[i]);
   free(names[i]);
}

However, operating system will reclaim the memory consumed by a process once the process exits, so in this simple example you don't have to worry, but I guess that this is only an illustration. I also assume that you don't care about user entering names longer than 10 characters which would write over the boundaries of allocated buffers.

Zdeslav Vojkovic
  • 14,391
  • 32
  • 45
  • Thanks.But please check my edit and tell me about what you think about that program. – Thokchom May 13 '13 at 15:05
  • How to take care of boundary then and not to over write the allocated memory? How can I make sure that it will not go more then 10chars? – devnp Feb 11 '14 at 19:41
  • see http://stackoverflow.com/questions/1621394/how-to-prevent-scanf-causing-a-buffer-overflow-in-c or http://stackoverflow.com/questions/17314796/max-string-length-using-scanf-ansi-c – Zdeslav Vojkovic Feb 11 '14 at 21:42
2

A separate block of memory will be allocated each time, since C does not perform any garbage collection and no free calls are done in between the malloc calls. The first three blocks are leaked since you're not keeping pointers to them.

You can find out for yourself that the pointers are all distinct using a simple printf call:

for (int i = 0; i < 4; i++) {
    ptr = malloc(10);     // no need for sizeof(char), it's 1 by definition
    printf("%p\n", ptr);
}

This should print four distinct numbers.

Fred Foo
  • 355,277
  • 75
  • 744
  • 836
  • Please spare a moment to check the edit to my question as well. – Thokchom May 13 '13 at 15:05
  • @Thokchom: correction, misread your code. It seems safe except that you don't `free` before exiting (and you're also using more `malloc` calls than necessary; why copy from one `malloc`'d buffer to another?). – Fred Foo May 13 '13 at 15:08
  • What else to `free` before exiting?Haven't I freed all memory blocks that `ptr` was ever associated with it in the program?Can you tell me what else to free? – Thokchom May 13 '13 at 15:11
  • Oh,I think I am getting you,I am not freeing the memory allocated to the names right..those who are pointed to by pointers in the array?If yes,will I need a loop to free those memory blocks pointed by each pointer in the pointer array? – Thokchom May 13 '13 at 15:13
  • @Thokchom: each `names[i]`. That doesn't matter much on most operating systems, but it's still good practice to match every `malloc` with one `free`. When in doubt, just count the number of calls to each in the program. – Fred Foo May 13 '13 at 15:13
  • @Thokchom: you can `free` them in the `printf` loop. After the `printf`, they're no longer needed. – Fred Foo May 13 '13 at 15:13
  • Is there any way to free all the memory blocks pointed by the pointers in the pointer array in **one go**?Or I need a loop for this? – Thokchom May 13 '13 at 15:14
  • @Thokchom: you need a loop. There's no standard shorthand. – Fred Foo May 13 '13 at 15:16
  • Thanks.Last thing.Why you said `why copy one malloc'd buffer to another..`?Actually, my intent is to allocate just as much memory each time as there are characters in the names entered,which may vary.I have used `strlen`for this.Can you suggest a better approach where I won't need to copy.I really feel **copying** each time is an unnecessary overhead.But if I want to allocate only the `strlen+1` number of bytes,is there an alternative to copying?Tell me this only please.Last thing.If possible post the code snippet for it. – Thokchom May 13 '13 at 15:19
  • @Thokchom: you could use `realloc`. But be aware that most implementations of that don't actually shrink buffers unless they become much smaller than the initial allocation, and requesting ten bytes from `malloc` or `realloc` may actually use up (say) a hundred. – Fred Foo May 13 '13 at 15:22
  • Thanks a lot.You have no idea how much i learnt from this discourse with you. – Thokchom May 13 '13 at 15:23
2

This will cause a memory leak assuming each call is successful, all but the memory allocated to the last malloc will be inaccessible. You either need to keep a pointer to each block allocated by malloc or you need to free between successive calls. so yes, malloc will attempt to allocate distinct block of memory for each call. A quick test could be as follows:

#include <stdio.h>
#include <stdlib.h>

int main()
{
    char *ptr;
    ptr=malloc(sizeof(char)*10);
    printf("%p\n", ptr);
    ptr=malloc(sizeof(char)*10);
    printf("%p\n", ptr);
    ptr=malloc(sizeof(char)*10);
    printf("%p\n", ptr);
    ptr=malloc(sizeof(char)*10);
    printf("%p\n", ptr);
}

This will print out the address allocated each time. If you added a free between each call to malloc you may end up with the same pointer, a quick test demonstrated this behavior.

Shafik Yaghmour
  • 154,301
  • 39
  • 440
  • 740
1

Your second part is right: it's a memory leak.

nvoigt
  • 75,013
  • 26
  • 93
  • 142
1

A new block of memory is allocated each time you call malloc. This will create a memory leak as you have no way to reference the first three allocations to free them.

gareththegeek
  • 2,362
  • 22
  • 34
1

You can check memory leaks by using valgrind,its very useful in detecting memory leaks.

valgrind ./progr
Coffee_lover
  • 545
  • 6
  • 15