2

So I am currently writing a function that can output data to an array ready for export to text. Function works fine when collecting variables but an getting the "Thread 1: EXC_BAD_ACCESS (code=1, address=0x0)" error within Xcode and don't know how to debug this. I have tried using calloc to assign memory to the array and using address locations but am still getting similar error messages and using address locations just don't work.

Has anyone got any suggestions for how I can solve this? The error code is showing itself on the first line of the for loop and this function is running as part of a larger function.

void LamVelProf(double dP, double L, double d, double mu)
{
    double *r = malloc(sizeof(r)); //Point radius from centreline
    double *R = malloc(sizeof(R)); //Absolute pipe radius
    double *vx = malloc(sizeof(vx));
    double *gvx = malloc(sizeof(gvx));
    double *offset = malloc(sizeof(offset));
    
    double **profile[7500][4];
    
    **profile = calloc((7500*4), sizeof(double));
    //double **profile = calloc((7500*4), sizeof(double));
    int *i = malloc(sizeof(i));
    
    *R = d/2; //Setting the boundary condition
    *offset = 0.001;
    *i = 0;
    
    for(*r = 0; *r < (*R + (*offset/2)); *r = (*r)+(*offset))
    {
        **profile[*i][0] = *r;
        LamVelProCalc(dP, L, d, mu, *r, vx);
        **profile[*i][1] = *vx;
        LamGenProCalc(*r, d, gvx);
        **profile[*i][2] = *gvx;//Results from general profile
        **profile[*i][3] = *i+1;
        
        ++*i; //Increasing count by 1
    }
    printf("%i rows generated\n", *i);
    
    free(r);
    free(R);
    free(offset);
    
    int *row = malloc(sizeof(row));
    int *col = malloc(sizeof(col));
    
    for(*row = 0; *row < *i + 1; *row = *row + 1)
    {
        for(*col = 0; *col < 4; *col = *col + 1)
        {
            printf("%f", **profile[*row][*col]);
            
            if(*col == 3)
            {
                printf("\n");
            }else{
                printf("\t");
            }
        }
    }
}
Matt C
  • 63
  • 5
  • 2
    What is going on here? `**profile = calloc((7500*4), sizeof(double));` What is this definition trying to achieve? `double **profile[7500][4]` Your use of pointers here is completely bizarre and makes almost no sense, it just massively complicates your code and provides zero gains. Don't `malloc()` an individual `double`, just have a regular `double` variable. – tadman Jun 29 '20 at 22:43
  • `(*r)+(*offset)` why are you dereferencing the pointer r if you want to increment it ? – sshmaxime Jun 29 '20 at 22:45
  • @tadman I am allocating memory to the array using calloc as I read on another post that this error may be coming from the size of the array not being able to fit onto the stack – Matt C Jun 29 '20 at 22:46
  • The more I look at this code the more I see pathological habits that you need to stop right now. `int *row = malloc(sizeof(row));` Treat allocations seriously in C or they will utterly ruin you. Every single `malloc` needs a corresponding `free`, and you're leaking memory each time this function is called because `row` and `col` are never released, not that they should be pointers in the first place. – tadman Jun 29 '20 at 22:47
  • @Maxime I was following what Xcode was telling me was the right way to perform the operation – Matt C Jun 29 '20 at 22:48
  • 2
    If you're learning C, you urgently need a better [C reference book](https://stackoverflow.com/questions/562303/the-definitive-c-book-guide-and-list). This code is so far into the weeds it's downright alarming. – tadman Jun 29 '20 at 22:48
  • 1
    I think you're misinterpreting what XCode is saying. It's probably trying to do its best given the code you have, and the code you have is vastly different from the C code compilers are used to seeing. – tadman Jun 29 '20 at 22:49
  • At the end of the day never rely on what your IDE says for most bug resolution. – sshmaxime Jun 29 '20 at 22:52
  • `"Thread 1: EXC_BAD_ACCESS (code=1, address=0x0)"` generally points to a pointer that is being abused. You obviously cannot access memory in the system reserved area way down at the bottom of the memory range. – David C. Rankin Jun 29 '20 at 23:11
  • `double *r = malloc(sizeof (r))` allocates just enough space to hold a pointer to a double. (which quite possibly is not large enough to hold a double). If you instead did the more idiomatic `double *r = malloc( sizeof *r )`, you would allocate enough space to hold one double. But in that case you might as well just do `double r`. If you want to allocate space for n doubles, you need `double *r = malloc( n * sizeof *r)` – William Pursell Jun 29 '20 at 23:14

2 Answers2

3

I've had to aggressively de-pointerize this code to bring it back into the realm of understandability, and the end result is this:

void LamVelProf(double dP, double L, double d, double mu)
{
    double R = d/2;
    double offset = 0.001;
    
    double profile[7500][4];
    
    int i = 0;
    
    for (double r = 0; r < (R + (offset/2)); r += offset) {
        double vx = 0.0; // Initialize appropriately
        double gvx = 0.0;

        profile[i][0] = r;

        // No idea what this does, or why the return value is ignored
        LamVelProCalc(dP, L, d, mu, r, vx);

        profile[i][1] = vx;

        // No idea what this does, or why the return value is ignored
        LamGenProCalc(r, d, gvx);

        profile[i][2] = gvx;//Results from general profile
        profile[i][3] = i+1;
        
        ++i; //Increasing count by 1
    }

    printf("%i rows generated\n", i);
    
    for(int row = 0; row < i + 1; ++row)
    {
        for(int col = 0; col < 4; ++col)
        {
            printf("%f", profile[row][col]);
            
            if (col == 3) {
                printf("\n");
            } else {
                printf("\t");
            }
        }
    }
}

As you can see there's two function calls buried in there that should probably have pointer arguments, my guess is vx and gvx are intended to be manipulated by that function. In C it is common to use use pointers to manipulate external variables, so a pointer argument almost always means "array" or "mutable argument" depending on context.

In other words I'd expect to see:

LamVelProCalc(dP, L, d, mu, r, &vx);

Or even better:

double vx = LamVelProCalc(dP, L, d, mu, r);

Where that value is explicitly returned instead.

This should compile and run without crashing now, though note the above mentioned issues.

When it comes to compiler suggestions to fix a problem, remember to take them all under advisement. At the end of the day you're the programmer, not the compiler, and not every educated guess it makes will be a valid interpretation of the problem at hand. If you unwaveringly follow the compiler's advice it may lead you down really, really strange paths, as perhaps has happened here.

As a note, having variables r and R is borderline programmer abuse. Please don't do this.

Another thing to keep in mind is your rather arbitrary use of 7500 here. Is that just a wild guess as to how many entries you'll need? It's almost always better to compute that, you know how the for loop will run in advance so you can do the math, and allocate accordingly.

If it is a limit you've arrived at through some other method it's worth using a #define to indicate as such, like:

#define MAX_PROFILE_ENTRIES 7500

Where it's now clear what the meaning behind that number is.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • Thanks so much for your help. The implicit calls are just referring to different calculations for the velocity profile for laminar flow. Although the functions do have returns for vx and gvx respectively, the compiler was acting funny with me deallocating the value after return the value. Sorry about the variable names, was just doing the shorthand way of saying point radius and pipe radius – Matt C Jun 29 '20 at 23:14
  • Not sure what "deallocating the value after return the value" means in this particular context, but it sounds like you're referencing something that fell out of scope. – tadman Jun 29 '20 at 23:15
  • The reason I mention variable names like that is it frustrates code review, and also bucks the convention of variables starting out lower-case. `R` is presumed to be a `#define` macro based on its name alone. – tadman Jun 29 '20 at 23:16
  • Oh ok - I didn't know about R being presumed to be a macro. Meant freeing the variable after returning said value if that clarifies things? – Matt C Jun 29 '20 at 23:19
  • C doesn't have strict conventions like other languages, but modern C has picked up habits like that from other languages. Just something to keep in mind when picking variable names, and so on. – tadman Jun 29 '20 at 23:20
  • I don't know what "freeing the variable after returning" means since nothing gets freed automatically in C, something must explicitly do that, and a function can't do it after it has exited. When I say something "fell out of scope" I mean something completely different than `free()`, which is with respect to dynamic allocation. – tadman Jun 29 '20 at 23:20
0

So after a lot of head scratching and realising I didn't need malloc because my array would definitely never reach the 125000 elements. Thanks to @tadman for helping with that. Here's the final program which is designed to be called from a menu function:

//
//  02g1LamVelPro .c
//  Process Model (MacOS Version)
//
//  Created by --- on 30/06/2020.
//  Copyright © 2020 ---. All rights reserved.
//

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

#define maxstrlen 128

//Declaring global variables and allocating memory
    //Function Output
double profile; //Array of doubles
    //Calculation Variables
double dP;
double L;
double d;
double mu;
double r;
    //Miscellaneous Variables


void LamVelProVar(double *dP, double *L, double *d, double *mu)
{
    //Declaring input variables
    char pres[maxstrlen];
    char len[maxstrlen];
    char dia[maxstrlen];
    char visc[maxstrlen];
    
    printf("Fluid pressure loss (Pa) = ");
    *dP = atof(fgets(pres, sizeof(pres), stdin));
    
    printf("Pipe length (m) = ");
    *L = atof(fgets(len, sizeof(len), stdin));
    
    printf("Pipe diameter (mm) = ");
    *d = atof(fgets(dia, sizeof(dia), stdin));
    
    *d = (*d)*0.001;
    
    printf("Fluid viscosity (cP) = ");
    *mu = atof(fgets(visc, sizeof(visc), stdin));
    
    *mu = (*mu)*0.001;
    
    fflush(stdout);
}

double LamVelCalc(double dP, double L, double d, double mu, double r, double *v_x) 
{
    //Calculation of the theoretical velocity profile with the flow possessing laminar characteristics
    double frac1;
    double frac2;
    double frac3;
    
    frac1 = (dP/L);
    
    frac2 = pow(d,2);
    frac2 = (frac2)/(16*mu);
    
    frac3 = 2*r;
    frac3 = (frac3)/d;
    frac3 = pow(frac3, 2);
    frac3 = 1 - (frac3);
    
    *v_x = frac1 * frac2;
    *v_x = (*v_x) * frac3;
    
    return *v_x;
}

double LamGenCalc(double r, double d, double *func)
{
    //Calculation of the general velocity profile with the flow possessing laminar characteristics
    *func = 2*r;
    *func = (*func)/d;
    *func = pow(*func, 2);
    *func = 1 - (*func);
    return *func; //Returns v/v_max
}

double **LamVelProfCalc(double dP, double L, double d, double mu) 
{
    char display[maxstrlen];
    
    double v_x = 0;
    double offset = 0.0001;
    //Calculating number of rows for the profile results matrix
    double prad = d/2;
    
    int whildisp = 1;
    int rows = ((prad)/ (offset)) + 1;
    printf("%i rows required\n", rows); 
    double profile[rows][3];
    
    int i = 0;
    for(double r = 0.0; r < (prad + (offset/2)); r += offset)
    {
        profile[i][0] = r; //Displaying point radius
        profile[i][1] = LamVelCalc(dP, L, d, mu, r, &v_x); //Calculating point velocity
        profile[i][2] = LamGenCalc(r, d, &v_x); //Calculating 
        //profile[i][3] = i + 1;
        ++i;
    }
    printf("%i rows successfully generated\n\n", i);
    
    while(whildisp == 1)
    {
        printf("Do you want to display the generated data? ");
        fgets(display, sizeof(display), stdin);
        switch(display[0])
        {
            case '1':
            case 'Y':
            case 'y':
                printf("Displaying data\n");
                printf("Inputted variables:\n");
                printf("dP =\t%.3f\tPa\n", dP);
                printf("L =\t%.3f\tm\n", L);
                printf("d =\t%.1f\tmm\n", d*1000);
                printf("mu =\t%.3f\tPa.s\n", mu);
                printf("v_max =\t%.3f\tm/s\n\n", LamVelCalc(dP, L, d, mu, 0, &v_x));
                
                printf("r (m)\tv_x (m/s)\tv/v_max\n");
                int row = 0;
                int col = 0;
                for(row = 0; row < i; ++row)
                {
                    for(col = 0; col < 3; ++col)
                    {
                        printf("%.5f", profile[row][col]);
                        if(col == 2)
                        {
                            printf("\n");
                        }else{
                            printf("\t");
                        }
                    }
                }
                whildisp = 0;
            break;
            case '0':
            case 'N':
            case 'n':
                whildisp = 0;
            default:
                printf("Input not recognised.\n");
            break;
        }
    }
    
    return profile;
}

void LamVelPro()
{
    //Main Function
    char ContCond[maxstrlen];
    
    int whilmain = 1;
    printf("Laminar flow velocity profile\n");
    
    while(whilmain == 1)
    {
        //Variable declaration
        double dP;
        double L;
        double d;
        double mu;
        double r;
        //Data collection
        LamVelProVar(&dP, &L, &d, &mu);
        //Data manipulation
        LamVelProfCalc(dP, L, d, mu);
        
        //Ask for file write (Remember while loop)
        //...
        //Continue function
        int whilcont = 1;
        while(whilcont == 1)
        {
            printf("Do you want to continue? ");
            fgets(ContCond, sizeof(ContCond), stdin);
            switch(ContCond[0])
            {
                case '1':
                case 'T':
                case 'Y':
                case 't':
                case 'y':
                    whilcont = 0;
                break;
                case '0':
                case 'F':
                case 'N':
                case 'f':
                case 'n':
                    whilcont = 0;
                    whilmain = 0;
                break;
                default:
                    printf("Input not recognised\n");
                break;
            }
        }
    }
    fflush(stdout);
}

Matt C
  • 63
  • 5