I have an array of integer elements and whenever I try to copy (value of) one of those elements to an integer pointer, I receive a segmentation fault. The segmentation fault is coming from the integer pointers because if I use a regular integer (instead of int*), then there is no seg fault. I am trying to figure out why am I getting a seg fault for storing an integer value like *max = arr[0]. Can someone please clarify why it is illegal in C to do that?
int main (int argc, char **argv) {
int arr[] = {1000, 11, 445, 1, 330, 3000};
int len = sizeof(arr)/sizeof(arr[0]);
int *min;
int *max;
if (arr[0] > arr[1]) {
*max = arr[0]; <----Seg fault here.
*min = arr[1];
}else {
*min = arr[0];
*max = arr[1];
}
// Calculate min and max in array and stores those values in `min` and `max`
calculateMinMax (arr, len, min, max);
printf("Min Value:%d", *min);
printf("Max Value:%d", *max);
return 0;
}
Result in GDB
Program received signal SIGSEGV, Segmentation fault.
0x00005555555552bf in main (argc=1, argv=0x7fffffffe208) at minmaxarray.c:32
32 *max = arr[0];
(gdb)
CodePudding user response：
When you work with pointers, you always have to keep in mind the distinction between the pointer itself, versus the value that the pointer points to. And until you've set the pointer itself to point somewhere, the concept "the value that the pointer points to" is meaningless.
For a pointer variable int *max, when you say
max = ...
you're setting the pointer itself, and when you say
*max = ...
you're setting the value that the pointer points to.
But in the code you've posted, you've got lines like
*max = arr[0];
You're trying to set the value that the pointer points to, before you've set the pointer to point anywhere. As I said, this is meaningless, and it tends to result in a segmentation fault or other such fatal error.
As others have posted, if you want min and max to point to the certain elements in the array, you want something like
max = &arr[0];
If you just wanted *max to be a value fetched from the array, that's what
*max = arr[0];
might do, but first, you'd have to make sure max pointed somewhere. You could do
int temporary_storage_for_max;
max = &temporary_storage_for_max;
after which lines like
*max = arr[0];
would work. But in that case, it would be silly to use pointers at all. In that case, you might as well declare max as a plain int:
int max;
and then do things like
max = a[0];
But I'm guessing you do want min and max to be pointers, in which case, as mentioned,
max = &arr[0];
is the way to go.
Your calculateMinMax function probably needs work, too, but since you haven't shown that it's hard to say what it might need. (Conceivably, if calculateMinMax does the min/max-finding work and does it properly, you don't need these initial assignments of min and max at all.)
CodePudding user response：
You want to assign max to &arr[0], not the value of the first item in the array to the address in the pointer, as the pointer's value is currently undefined.
i.e.
max = &arr[0];
min = &arr[1];
etc.
CodePudding user response：
Your indirection is wrong, and ultimately misplaced. The mystery function calculateMinMax appears to desire to calculate the minimum, and maximum values of a given sequence, storing their results in out-arguments provided by address from the caller.
int main (int argc, char **argv) {
int arr[] = {1000, 11, 445, 1, 330, 3000};
int len = sizeof(arr)/sizeof(arr[0]);
// initialize these if you want, but it shouldn't
// be necessary if calculateMinMax does its job
// correctly
int min;
int max;
// note last two arguments passed are the *addresses*
// of our output arguments, in which to store the
// final min/max values from the sequence.
calculateMinMax (arr, len, &min, &max);
printf("Min Value:%d", min);
printf("Max Value:%d", max);
return 0;
}
Don't make this harder than it needs to be. Pointers point to things. Yours were not specified as pointing to anything. Doing the above is synonymous to this:
int main (int argc, char **argv) {
int arr[] = {1000, 11, 445, 1, 330, 3000};
int len = sizeof(arr)/sizeof(arr[0]);
int min;
int max;
int *pmin = &min;
int *pmax = &max;
calculateMinMax (arr, len, pmin, pmax);
printf("Min Value:%d", min);
printf("Max Value:%d", max);
return 0;
}
CodePudding user response：
This code snippet
if (arr[0] > arr[1]) {
*max = arr[0]; <----Seg fault here.
*min = arr[1];
}else {
*min = arr[0];
*max = arr[1];
}
does not make a sense. The pointers max and min are not initialized. So dereferencing them invokes undefined behavior.
Moreover you have the function calculateMinMax that has to determine the maximum and minimum elements in the array.
So instead of the above code snippet just write
calculateMinMax (arr, len, &min, &max);
Pay attention to that you need to pass the pointers min and max to the function by reference through pointers to them. Otherwise the function will deal with copies of values of the pointers and changing the copies will not change the original pointers.
The function can be declared and defined the following way
void calculateMinMax( const int a[], size_t n, int **min, int **max )
{
*min = ( int * )a;
*max = ( int * )a;
for ( size_t i = 1; i < n; i )
{
if ( **max < a[i] )
{
*max = ( int * )( a i );
}
else if ( a[i] < **min )
{
*min = ( int * )( a i );
}
}
}
Here is a demonstration program.
#include <stdio.h>
void calculateMinMax( const int a[], size_t n, int **min, int **max )
{
*min = ( int * )a;
*max = ( int * )a;
for (size_t i = 1; i < n; i )
{
if (**max < a[i])
{
*max = ( int * )( a i );
}
else if (a[i] < **min)
{
*min = ( int * )( a i );
}
}
}
int main( void )
{
int arr[] = { 1000, 11, 445, 1, 330, 3000 };
size_t len = sizeof( arr ) / sizeof( arr[0] );
int *min;
int *max;
calculateMinMax( arr, len, &min, &max );
printf( "Min Value: %d\n", *min );
printf( "Max Value: %d\n", *max );
}
The program output is
Min Value: 1
Max Value: 3000