Segmentation fault when copying the value from an array element to an integer pointer-CodePudding

I was writing a test code in C and came across something which didn't make complete sense to me. I have an array of integer elements and whenever I try to copy (value of) one of those elements to an integer pointer, I get 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 segmentation fault. I am trying to figure out why am I getting a segmentation fault for storing an integer value like *max = arr[0].

Can someone please clarify why it is seg faulting? Since this is not a final code, you may find the comparison arr[0] > arr[1] little absurd. Please kindly ignore that. My goal is to figure out the reason for segmentation fault.

    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;
      
      // Please ignore the reasoning behind this comparison since
      // it is simply a test code in progress.
      // My goal is to find out why this assignment is seg faulting
      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：

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