1

Trying to understand memory allocation in C. Facing issue while trying to create two arrays using pointer to integers. Kindly have a look at the below code:

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

int main() {

    int *a;
    int *b;

    for (int i = 0; i<4;i++)
    {
        printf("Enter value \n");
        a[i]=(int *)malloc(sizeof(int));
        b[i]=(int *)malloc(sizeof(int));
        scanf("%d",&a[i]);
        scanf("%d",&b[i]);
    }
    for (int i =0;i<4;i++)
    {
      printf("%d = %x\n  ",a[i],&a[i]);

    }
    for (int i =0;i<4;i++)
    {
     printf("%d = %x\n  ",b[i],&b[i]);

    }

    return 0;
}

I am working with C11 on CLion. Facing below error on runtime. Can someone please explain what is wrong with this code ?

Enter value 

Process finished with exit code 11

b is being shown as NULL during debugging

"b is being shown NULL during debugging"

UPDATE: Tried on another IDE, where "a" itself is not being allocated any memory. It directly gives me segmentation fault.

UPDATE 2: Changing:

int *a;
int *b;

to

int *a = NULL;
int *b = NULL;

at least stops how this code is behaving. It gives me segmentation fault as soon as I try to allocate memory to a[i] (Which is wrong, now I get).

Manoj
  • 853
  • 1
  • 7
  • 28
  • `a[i]=(int *)malloc(sizeof(int));`: `a` doesn't point anywhere. It is not initialized. You want `a =(int *)malloc(sizeof(int) * 4);`. But also other issues. – Jabberwocky Mar 19 '18 at 07:08
  • 1
    You should alloc memory for pointers as well. – Frederik.L Mar 19 '18 at 07:09
  • 1
    You need to go back to whatever book or tutorial you're following or reading, and take a close look on what it tells you about pointers and dynamic allocation. What you're doing is wrong on many levels. – Some programmer dude Mar 19 '18 at 07:10
  • If I remove "b" completely from the above code, it allocates memory for array "a" for 4 integers and prints the entered or scanned values. Issue only comes once I try to allocate memory for another array. – Manoj Mar 19 '18 at 07:13
  • You should never cast `malloc`. – WedaPashi Mar 19 '18 at 07:22

3 Answers3

3

While the code do allocate memory for four int values, it does not work as you expect.

If we start from the beginning:

int *a;

This defines (and declares) a variable a which is a pointer to an int. You do not initialize it, which means its value is indeterminate (and will seem totally random). Dereferencing this (with e.g. a[i]) leads to undefined behavior.

Then

for (int i = 0; i<4;i++)
{
    a[i]=(int *)malloc(sizeof(int));
    ...
}

If you properly initialized a it would already point to memory for four int values. You could see a as an array, and you don't need to allocate each member of the array separately. This is indicated by the types, a[i] is of type int, not int *.

The "best" solution is to not use pointers or dynamic allocation at all, and instead use plain arrays:

int a[4];

for (int i = 0; i<4;i++)
{
    printf("Enter value \n");
    scanf("%d",&a[i]);  // No allocation needed, a[i] already exists
}

If you must use pointers and dynamic allocation, then allocate memory enough for four int elements, and make a point to the first byte:

int *a = malloc(sizeof(int) * 4);

for (int i = 0; i<4;i++)
{
    printf("Enter value \n");
    scanf("%d",&a[i]);  // No allocation needed, a[i] already exists
}

There are a few other problems as well, but start with these.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Thanks for your answer. I believe there must be some other way to implement it. I want to allocate memory only till user wants to quit the program. "4" integers I only took for the example. For ex. user is typing and memory should be allocated until he enters "Q". – Manoj Mar 19 '18 at 08:12
  • 1
    @Manoj Then read about [`realloc`](http://en.cppreference.com/w/c/memory/realloc). – Some programmer dude Mar 19 '18 at 08:14
1

There is some confusion in your code:

  • you do not allocate the arrays for a and b to point to. a and b are uninitialized, dereferencing them has undefined behavior. a shows as NULL in the debugger, but the C Standard does not guarantee that. a could have any value, including trap representations that would cause undefined behavior just by reading it.
  • you store a pointer to int into a[i]: the compiler should issue a diagnostic about the type mismatch. It is unfortunate that this not be a fatal error IMHO, but ignoring compiler warnings is always a bad idea.
  • the return value of malloc() is cast as (int *). This is necessary in C++, but considered bad style in C.
  • the third argument in printf("%d = %x\n", a[i], &a[i]); is inconsistent with the conversion specifier: if you want to print the value in hex, use printf("%d = %x\n", a[i], a[i]);, if you want to print the address, use the %p conversion specifier: printf("%d at %p\n", a[i], (void*)&a[i]);

To play with malloc(), you should just allocate the int arrays, check for allocation success and free the memory after use:

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

int main() {
    int *a;
    int *b;

    a = malloc(sizeof(*a) * 4);
    b = malloc(sizeof(*b) * 4);
    if (a == NULL || b == NULL) {
        fprintf(stderr, "cannot allocate memory\n");
        exit(1);
    }

    for (int i = 0; i < 4; i++) {
        printf("Enter 2 values\n");
        if (scanf("%d%d", &a[i], &b[i]) != 2) {
            fprintf(stderr, "invalid input\n");
            exit(1);
        }
    }
    for (int i = 0; i < 4; i++) {
        printf("  %d = %x\n", a[i], a[i]);
    }
    for (int i = 0; i < 4; i++) {
        printf("  %d = %x\n", b[i], b[i]);
    }
    free(a);
    free(b);
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • As soon as I marked int *a = NULL; It stops working. Moreover, without NULL, if I print the "printf("%p\n",*(&a+1)-*(&a));" It prints the size that is accessible for "a" i.e. 0xffffe000462fd622. Thanks.!! – Manoj Mar 19 '18 at 08:38
  • @Manoj: with `int *a = NULL;`, your code has undefined behavior of the first kind: easy to spot because you are dereferencing the null pointer directly, which causes a segmentation fault on most current operating systems. Run with a debugger and the offending code will be easy to spot. With `int *a;`, the code has undefined behavior of the third kind: what actually happens depends on many factors, including alien influence... It may not crash immediately, but who knows what side effects may have been triggered... Harder to debug. – chqrlie Mar 19 '18 at 21:18
0

You need to add these lines, because you need to create memory so that you can assign values later:

  int **a = (int **)malloc(sizeof(int *) * 4);
  int **b = (int **)malloc(sizeof(int *) * 4);

You are assigning pointer values to elements of a. Therefore a should be pointer to pointer. Complete code:

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

int main ()
{

  int **a = (int **)malloc(sizeof(int *) * 4);
  int **b = (int **)malloc(sizeof(int *) * 4);

  for (int i = 0; i < 4; i++)
    {
      printf ("Enter value \n");
      a[i] = (int *) malloc (sizeof (int));
      b[i] = (int *) malloc (sizeof (int));
      scanf ("%d", &a[i]);
      scanf ("%d", &b[i]);
    }
  for (int i = 0; i < 4; i++)
    {
      printf ("%d = %x\n  ", a[i], &a[i]);

    }
  for (int i = 0; i < 4; i++)
    {
      printf ("%d = %x\n  ", b[i], &b[i]);

    }

  return 0;
}

Output:

Enter value 
4
5
Enter value 
1
2
Enter value 
6
7
Enter value 
8
9
4 = 165d010
  1 = 165d018
  6 = 165d020
  8 = 165d028
  5 = 165d040
  2 = 165d048
  7 = 165d050
  9 = 165d058
Syed Waris
  • 1,056
  • 1
  • 11
  • 16
  • In the last program, `scanf("%d", &a[i]);` should be replaced with `scanf("%d", a[i]);` and `printf("%d = %x\n ", a[i], &a[i]);` with `printf("%d = %x\n ", a[i][0], a[i][0]);`. As you say, memory allocation for this is completely unnecessary and leads to confusing code. – chqrlie Mar 19 '18 at 07:33
  • 1
    I don't see the need to allocate an "array" of *pointers*? – Some programmer dude Mar 19 '18 at 07:37