Why am I getting a segmentation fault? C programming

Question

Here's my code that I'm getting a seg fault at. I'm pretty sure it has to do with the pass-by-reference, but it confuses me and I'm not sure if I'm doing it right.

#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include "utils.h"
#define PI 3.1415926535897932384626433832795

int circleStatistics(double radius, double *diameter, double *circumference, double *area){
  *diameter = radius * 2;
  *circumference = PI * radius * 2;
  *area = PI * radius * radius;
  if (radius <= 0 || diameter == NULL || circumference == NULL || area == NULL)
    printf("An error has occured\n");
    return 1;
  }else{
    return 0;
  }
}

Now here is the code that I'm using to call the function and test it.

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

int main(int argc, char **argv){
  // Tests circleStatistics
  double radius = 3;
  double *diameter = NULL, *circumference = NULL, *area = NULL;
  circleStatistics(radius, diameter, circumference, area);
  printf("Expected output: radius = 3, diameter = 6, circumference ~= 18.849555, area ~= 28.2743339\n");
  printf("Actual output: radius = %.0f, diameter = %.0f, circumference ~= %.7f, area ~= %.7f\n", radius, *diameter, *circumference, *area);
}

You think the problem is in the way you pass the parameters, but you choose to omit that part? You also test for the pointers being `NULL`, but after you have de-referenced them? — David Heffernan, Oct 13 '14 at 18:54
your `if(` statement should be the first line of the function, before you perhaps de-reference potentially NULL pointers. — JohnH, Oct 13 '14 at 18:54

score 1 · Answer 1 · answered Oct 13 '14 at 18:54

1

You are dereferencing the pointer arguments and after that you are checking, whether they are not NULL... try something like this:

#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include "utils.h"
#define PI 3.1415926535897932384626433832795

int circleStatistics(double radius, double *diameter, double *circumference, double *area){
  if (radius <= 0 || diameter == NULL || circumference == NULL || area == NULL)
    printf("An error has occured\n");
    return 1;
  }else{
    /* now I know, that neither of the arguments point to NULL... */
    *diameter = radius * 2;
    *circumference = PI * radius * 2;
    *area = PI * radius * radius;
    return 0;
  }
}

answered Oct 13 '14 at 18:54

V-X

2,979
18
28

1

This is the naive answer to the question. Yes, it takes the question at face value, and answers it. But it would be better to critique the design. Are the checks for `NULL` even needed? Isn't it better to expect the caller to follow the contract? That sort of discussion would make this a much better answer. – David Heffernan Oct 13 '14 at 19:01
@David Heffernan Interesting thought. IMO, the code should perform `if (diameter != NULL) *diameter = radius * 2;` and like-wise for the other 2 and then return `void`. – chux - Reinstate Monica Oct 13 '14 at 19:05
@chux Certainly open to debate. I'd take the C standard library as my guide here. It won't check for user failure to adhere to interface contract. So `strlen(NULL)` won't return an error. Of course, the function designer could specify that parameters are optional, indicated by passing `NULL`. But then the implementation here would be wrong. – David Heffernan Oct 13 '14 at 19:07

score 0 · Answer 2 · answered Oct 13 '14 at 19:18

You didn't show the calling code, but most likely you are passing NULL for one of the pointers. Your current code dereferences the pointers first, and then checks that they are NULL. You've got this the wrong way round – you need to check before dereferencing.

However, I think it's worth considering the whole design. This is a function that is passed a single input value and returns three output values. You'd ideally like to use the function return value, but do not because you need to return three distinct values. Hence the pointers to simulate pass-by-reference. And then you start worrying about the possibility of a caller passing NULL. All this makes your code complex.

You could simply ignore the possibility of the user passing invalid arguments and document the requirements to the caller. That would be a valid choice. For instance, many functions in the standard library do that. A good example is strlen which demands that you pass a pointer to a null-terminated string. And what happens if you do not meet that requirement is not specified.

Personally, I'd wrap the three output values into a struct and so bypass most of these issues:

struct CircleStats
{
    double diameter;
    double circumference;
    double area;
}

struct CircleStats CalcCircleStats(double radius)
{
    struct CircleStats stats;
    stats.diameter = radius * 2;
    stats.circumference = PI * radius * 2;
    stats.area = PI * radius * radius;
    return stats;
}

This leaves the possibility that radius is negative. Personally I would simply ignore that possibility. Document that the caller must supply a positive radius, and expect them to meet that requirement. If they cannot meet that requirement then something is wrong in the calling code and it is not reasonable to expect this code to be able to deal with it.

Why am I getting a segmentation fault? C programming

2 Answers2