Well... "What were you thinking?"
int *ptr_a, a[a_length];
Let's look at two lines and see if we can make sense of what you are doing:
ptr_a = calloc(a_length, 4);
ptr_a = a;
Line 1 above allocates a block of memory with calloc
sizing the block for a_length
members of 4-bytes
each for a total of 20-bytes
given a_length = 5;
. It then assigns the starting address for the new block of memory to ptr_a
.
Line 2 above, then assigns the address of the array a
(with automatic storage type) to ptr_a
overwriting the memory address for the block of memory you just allocated (thereby causing a memory leak, because there a no more references to the start of that new block meaning it can never be freed).
However, since ptr_a
now points to a memory address that was NOT previously allocated with malloc
, calloc
or realloc
, when you pass ptr_a
to free (ptr_a);
boom! SegFault.
Using Memory Allocated by calloc
for Storage
You do not need the VLA (Variable Length Array) a
at all. You allocate a block of memory sufficient to hold five integer values and assign the starting address of that block to ptr_a
. You simply use ptr_a
as you attempted to use a
, e.g.
#include <stdio.h>
#include <stdlib.h>
#define NELEMENTS 5 /* if you need a constant, #define one (or more) */
int countelem (int *a, int a_length)
{
int i, count = 0;
for (i = 0; i < a_length; i++)
if (a[i] != 0) {
printf ("element number: %d and element is: %d\n", i, a[i]);
count++;
} /* no ; following block closure */
return count;
}
int main (void) {
int count = 0, /* initialize all variables */
a_length = NELEMENTS,
*ptr_a = NULL;
ptr_a = calloc(a_length, sizeof *ptr_a); /* allocate block of mem */
if (ptr_a == NULL) { /* validate & handle error before using block */
perror ("calloc-ptr_a");
return 1;
}
ptr_a[0] = 1; /* assign value 1 as first value in block of mem */
count = countelem (ptr_a, a_length);
printf ("number of non zeroes %d\n", count);
free(ptr_a); /* you validated the block above, just free */
return 0;
}
Example Use/Output
$ ./bin/alloccount
element number: 0 and element is: 1
number of non zeroes 1
Memory Use/Error Check
In any code you write that dynamically allocates memory, you have 2 responsibilities regarding any block of memory allocated: (1) always preserve a pointer to the starting address for the block of memory so, (2) it can be freed when it is no longer needed.
It is imperative that you use a memory error checking program to insure you do not attempt to access memory or write beyond/outside the bounds of your allocated block, attempt to read or base a conditional jump on an uninitialized value, and finally, to confirm that you free all the memory you have allocated.
For Linux valgrind
is the normal choice. There are similar memory checkers for every platform. They are all simple to use, just run your program through it.
$ valgrind ./bin/alloccount
==7530== Memcheck, a memory error detector
==7530== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==7530== Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info
==7530== Command: ./bin/alloccount
==7530==
element number: 0 and element is: 1
number of non zeroes 1
==7530==
==7530== HEAP SUMMARY:
==7530== in use at exit: 0 bytes in 0 blocks
==7530== total heap usage: 1 allocs, 1 frees, 20 bytes allocated
==7530==
==7530== All heap blocks were freed -- no leaks are possible
==7530==
==7530== For counts of detected and suppressed errors, rerun with: -v
==7530== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
Always confirm that you have freed all memory you have allocated and that there are no memory errors.
Using the VLA for Storage
Conversely, if you did want to make use of your VLA, then there is no need to allocate memory with calloc
. It's a one or the other, but not both, proposition.
Regardless of whether storage is provided by the VLA or calloc
, you can still use ptr_a
to point to that storage. However, if storage is provided by the VLA, then don't allocate with calloc
, simply:
int a[a_length], *ptr_a = a;
That's all that is required to declare a VLA and then a pointer to the VLA. (note: the VLA values are indeterminate, so before use, you may want to include string.h
and then memset (a, 0, sizeof a);
)
If you use your variable-length array as storage instead of allocating with calloc
, your code simplifies to:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define NELEMENTS 5 /* if you need a constant, #define one (or more) */
int countelem (int *a, int a_length)
{
int i, count = 0;
for (i = 0; i < a_length; i++)
if (a[i] != 0) {
printf ("element number: %d and element is: %d\n", i, a[i]);
count++;
} /* no ; following block closure */
return count;
}
int main (void) {
int count = 0, /* initialize all variables */
a_length = NELEMENTS,
a[a_length], *ptr_a = a;
memset (a, 0, sizeof a); /* initialize a all zero */
ptr_a[0] = 1; /* assign value 1 as first element */
count = countelem (ptr_a, a_length);
printf ("number of non zeroes %d\n", count);
return 0;
}
(note: the addition of memset
as discussed above. Without it, any attempt to access the indeterminate values in a[1]
- a[4]
would result in Undefined Behavior (and funky values output as you found))
Simply removing a_length
and using NELEMENTS
instead removes the VLA and instead provides storage with a normal array which you can initialize when it is declared, e.g.
int count = 0, /* initialize all variables */
a[NELEMENTS] = {0},
*ptr_a = a;
...
count = countelem (ptr_a, NELEMENTS);
(the output is the same, but no need for string.h
or memset
or any need to run it through a memory/error check)
Enable Additional Warnings (-Wextra
at minimum)
At minimum, add -Wextra
for gcc/clang (though you should additionally add -pedantic -Wshadow
as well) for VS use /W3
. Do not accept code until it compiles cleanly -- without a single warning. Listen to what your compiler is telling you, read and understand the warnings. It will give you the precise line on which each problematic bit of code is found. Go fix it before going further. Simply resolving all warnings can and will eliminate a majority of the problems you will find yourself spending a lot of time resolving otherwise.
Look things over and let me know if you have any questions.