1

I am learning about dynamic memory allocation and having a problem while doing the exercise. I would like to allocate the memory for p. The following is the codes.

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

int main(void)
{
    int *p, i;

    p = (int *)malloc(7 * sizeof(int));
    if (p == NULL)
    {
        printf("Memory Allocation Errors\n");
        exit(1);
    }
    for(i=0; i<6; i++)
    {
        p++;
        *p = i;
        printf("%3d",*p);
    }
    printf("\n");
    p = p - 6;
    free(p); 
    p = NULL;
    return 0;
}

The running result is:0 1 2 3 4 5

However, when I change the code

p = (int *)malloc(7 * sizeof(int));

to this

p = (int *)malloc(6 * sizeof(int)); 

the running result would be

0  1  2  3  4  5
Error in `./memo2': free(): invalid next size (fast): 0x0000000001bc5010 ***
Aborted (core dumped)

I think for this code, p only need the memory of the size of 6 integers, could anyone give a explanation?

Another question is that why should p++ be ahead of *p = i? If I make *p = i before p++, the running result would be 0 0 0 0 0 0.

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
Jinlong
  • 77
  • 1
  • 2
  • 8
  • 3
    First of all, [don't cast the result of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). Secondly of all, you don't initialize the first item in the allocated memory. Thirdly, wouldn't it be *much* easier to either use array-indexing syntax instead of pointer arithmetic, or to have two pointers, one original that you pass to `free` and one that you do whatever arithmetic you want? – Some programmer dude Aug 04 '14 at 10:35
  • `p++;` move to after `printf("%3d",*p);` – BLUEPIXY Aug 04 '14 at 10:37
  • Use printf("\n p[%p]", p); after allocation and before free and check it – mahendiran.b Aug 04 '14 at 10:41
  • You wasted one slot 'p' itself. 'p' is incremented before first using. So this is a one-off error. – Danke Xie Aug 04 '14 at 11:10

6 Answers6

3

First question - you get an error when you malloc memory for 6 integers because in your for loop you increment the pointer before assigning a value. Therefore you leave the first address you allocated empty. If you assign a value and then increment the pointer this will solve your problem.

Second question - if you perform the printf after incrementing the pointer you won't print the value you just set. You need to assign the value, print it, then increment the pointer.

Here's your code with those changes:

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

int main(void)
{
   int *p, i;

   p = (int *)malloc(6 * sizeof(int));
   if (p == NULL)
   {
      printf("Memory Allocation Errors\n");
      exit(1);
   }
   for(i=0; i<6; i++)
   {
      *p = i;
      printf("%3d",*p);
      p++; 
   }
   printf("\n");
   p = p - 6;
   free(p); 
   p = NULL;
   return 0;
}

However, I would recommend you use array indexing syntax. As well as being easier to read, this means that you don't actually change the p meaning that when you free the memory it will be on the correct address. Here's an updated version of your code using array indexing:

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

int main(void)
{
   int *p, i;

   p = (int *)malloc(6 * sizeof(int));
   if (p == NULL)
   {
      printf("Memory Allocation Errors\n");
      exit(1);
   }
   for(i=0; i<6; i++)
   {
      p[i] = i;
      printf("%3d",p[i]);
   }
   printf("\n");
   free(p); 
   p = NULL;
   return 0;
}
sclarke81
  • 1,739
  • 1
  • 18
  • 23
3

Your code needs 7 "slots" to store 6 numbers because the very first slot is immediately skipped:

for(i=0; i<6; i++)
{
    p++; // <-- Skip to next "slot"
    *p = i;
    printf("%3d",*p);
}

So the first slot is never used since it's immediately skipped. To fix this, first work with the current pointer, then increment it:

p = malloc(6 * sizeof(int));
if (p == NULL)
{
   printf("Memory Allocation Errors\n");
   exit(1);
}
for(i=0; i<6; i++)
{
    *p = i;
    printf("%3d",*p);
    p++; // <-- Skip to next "slot"
}

Also, the pointer you pass to free must be exactly the same pointer you got from malloc. You can calculate the original value by undoing the additions you did in a loop, but it's easier to store the original value (less chance to screw up):

int *p, *orig_p, i;

orig_p = malloc(6 * sizeof(int));
if (orig_p == NULL)
{
   printf("Memory Allocation Errors\n");
   exit(1);
}
p = orig_p;
...
free(orig_p);
DarkDust
  • 90,870
  • 19
  • 190
  • 224
1

For your subquestion2, find the answer in below

for(i=0; i<6; i++)
{
    *p = i;
    printf("%3d",*p); //Fix1
    p++; //Fix2
}

ie,you should print the value *p before increment the pointer p.

mahendiran.b
  • 1,315
  • 7
  • 13
1

1) When you do like this

for(i=0; i<6; i++)
{
  p++; // here before assigning the value you are incrementing address of p(first location is waste)
  *p = i;
  printf("%3d",*p);
}

So it does not store value in first memory location(this location is wasted). it stores value from second location on wards. so it requires 7 location for 6 numbers.

2) If you make *p = i before p++, like below code-

for(i=0; i<6; i++) 
{
    *p = i; // you are assigning the value.
    p++; // here you are incrementing the value
    printf("%3d",*p); // it will print the value at the incremented loaction
}

For example- Consider starting address is 1000. when you do *p = i; i is assigned to 1000, p++; will increment the location to 1004. After incrementing the address you are printing the value. It may give you 0 or garbage values. because printf("%3d",*p); will print the value at 1004. you don't know what value is there. So avoid this types of method.

Try the following code-

p = (int *)malloc(6 * sizeof(int)); // Fix 1
if (p == NULL)
{
    printf("Memory Allocation Errors\n");
    exit(1);
}
for(i=0; i<6; i++)
{
    *p = i;
    printf("%3d",*p);
    p++; // Fix 2
}
printf("\n");
p = p - 6;
free(p); 
p = NULL;
Sathish
  • 3,740
  • 1
  • 17
  • 28
1

Try this.

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

int main(void)
{
 int *p, i;

 p = malloc(7 * sizeof(int)); /* Memory allocated for 7 intergers */
 if (p == NULL)
 {
  printf("Memory Allocation Errors\n");
  exit(1);
 }
 for(i=0; i<=6; i++) /* Assign values */
 {
  // This is a better way of assignment, easy to understand
  // *(p+i) = i OR p[i] = i
  p[i] = i; 
  printf("%3d",p[i]); 
 }
 printf("\n");
 // p = p - 6; /* Not required */ 
 free(p); /* Free the pointer */
 // p = NULL; /* Not required */
 return 0;
}

The output will be:

0 1 2 3 4 5 6

ani627
  • 5,578
  • 8
  • 39
  • 45
  • Your solution is correct, but you should explain what you changed and why. Without such an explanation, the OP has to figure what you changed and guess why you did it (which might be impossible for a newbie). – DarkDust Aug 04 '14 at 11:42
  • Thanks for suggestion DarkDust, I have added more comments. – ani627 Aug 04 '14 at 13:04
  • Could still use a bit more explanation (the main point I'd like to have seen explained is that array syntax avoids some of the problems the OP had), but still +1 for the effort ;-) – DarkDust Aug 04 '14 at 13:19
0

You have p, that's one slot. Then you increment p six times, that's six more slots. So you need seven slots in total.

David Schwartz
  • 179,497
  • 17
  • 214
  • 278