How to use dynamically allocated memory in C

Question

I have been developing a block in Scilab/Xcos (open source variant of Matlab/Simulink) with C language usage. For that purpose the Scilab/Xcos offers a special block called CBLOCK4. As soon as I place this block into the simulation a C language stub is automatically generated. Part of that stub is a template for a function which implements the behavior of the block. This function accepts among its parameters pointer to a scicos_block structure:

typedef struct {
  int nevprt;
  voidg funpt ;
  int type;
  int scsptr;
  int nz;
  double *z;
  int noz;
  int *ozsz;
  int *oztyp;
  void **ozptr;
  int nx;
  double *x;
  double *xd;
  double *res;
  int nin;
  int *insz;
  void **inptr;
  int nout;
  int *outsz;
  void **outptr;
  int nevout;
  double *evout;
  int nrpar;
  double *rpar;
  int nipar;
  int *ipar;
  int nopar;
  int *oparsz;
  int *opartyp;
  void **oparptr;
  int ng;
  double *g;
  int ztyp;
  int *jroot;
  char *label;
  void **work;
  int nmode;
  int *mode;
} scicos_block;

The item work is probably intended to be used for storage of the address at which another address is stored at which the internal state variables of the block are stored. I have attempted to implement the Scilab/Xcos block with the work item usage:

#include "scicos_block4.h"

#define U  ((double *)GetRealInPortPtrs(block, 1))
#define Y  ((double *)GetRealOutPortPtrs(block, 1))

// parameters
#define Tu (GetRparPtrs(block)[0])
#define Td (GetRparPtrs(block)[1])
#define T  (GetRparPtrs(block)[2])


void Ramp(scicos_block *block, int flag)
{
 
  double *target;
  double *inputDelta;
  double *out;

  if(flag == 4) 
  {
    /* init */
    if((*(block->work) = (double *)scicos_malloc(sizeof(double)*3)) == NULL)
    {
        set_block_error(-16);
        return;
    }
        
    target      = (double*)(*(block->work));
    inputDelta  = (double*)(*(block->work + 1));
    out         = (double*)(*(block->work + 2));

    *target     = 0;
    *inputDelta = 0;
    *out        = 0; 

  }
  else if(flag == 1) 
  {
    /* output computation */ 
    if(U[0] != *target)
    {
        *target = U[0];
        
        if(*target - Y[0] < 0)
        {
            *inputDelta = Y[0] - *target;
        }
        else
        {
            *inputDelta = *target - Y[0];
        }
    }
    
    if(*target > Y[0])
    {
        *out += *inputDelta*T/Tu;
        if(*out > *target)
        {
            *out = *target;
        }
    }
    else if(*target < Y[0])
    {
        *out -= *inputDelta*T/Td;
        if(*out < *target)
        {
            *out = *target;
        }
    }
    
    Y[0] = *out;  
  } 
  else  if (flag == 5) 
  {
    /* ending */
    scicos_free(*(block->work));

  }

}

I am able to successfully compile the block containing this code but in case I start the simulation it crashes. I have doubts regarding the dynamic allocation of the memory and the way how I work with it.

Please could anybody take a look at my code (namely the part in the body of the if with flag == 4) and tell me what I am doing wrong?

Answer 1

As mentioned in user 4386427's answer, (double*)(*(block->work + 1)) should be (double*)(*(block->work)) + 1. This is especially important if sizeof(double) and sizeof(void*) have different values. (On a typical 64-bit system they would have the same value, but on a typical 32-bit system they would have different values.)

Another serious problem is that when flag == 1, the target, inputDelta and out variables contain uninitialized values but are being dereferenced. The variables need to be assigned in the else if (flag == 1) block before they can be dereferenced in the same way as in the if (flag == 4) block.

I think the code would be cleaner if a struct type was defined to hold the working data:

struct Ramp_work
{
    double target;
    double inputDelta;
    double out;
};

void Ramp(scicos_block *block, int flag)
{
    struct Ramp_work *work;

    if (flag == 4)
    {
        /* init */
        work = scicos_malloc(sizeof(*work));
        *block->work = work;
        if (work == NULL)
        {
            set_block_error(-16);
            return;
        }
        work->target = 0;
        work->inputDelta = 0;
        work->out = 0;
    }
    else if (flag == 1)
    {
        /* output computation */
        work = *block->work;
        if (U[0] != work->target)
        {
             /* etc. */
        }
        /* etc. */
    }
    else if (flag == 5)
    {
        /* ending */
        scicos_free(*block->work);
    }
}

Answer 2

This line

target      = (double*)(*(block->work));

is correct but these lines

inputDelta  = (double*)(*(block->work + 1));
out         = (double*)(*(block->work + 2));

are wrong.

(*(block->work) will give you the pointer to the first of the three malloc'ed doubles.

However, you don't get the next double like you did - you are adding one too early. Change it like:

(double*)(*(block->work + 1));   // Wrong !!
(double*)(*(block->work)) + 1;   // Correct !!
^^^^^^^^^^^^^^^^^^^^^^^^^   ^
Get pointer to malloced     Then add 1 to get to the second malloc'ed double
memory (i.e. the first
malloc'ed double) and use 
it as a double-pointer

Or simply do it like:

target      = (double*)(*(block->work));
inputDelta  = target + 1;
out         = target + 2;

BTW: You don't need all the casting. Just remove it.

Edit: In a comment OP tells that it still crashes

The posted code doesn't tell us whether block->work has been initialized before calling Ramp. If it hasn't been initialized before then doing *(block->work) will be illegal - and may cause a crash.

So maybe the code is missing something like:

/* init */
block->work = malloc(sizeof *block->work);  // Add this line
if((*(block->work) = (double *)scicos_malloc(sizeof(double)*3)) == NULL)