problems with functions with variable numbers of agruments

Hello

I’m in the process of writing a timeline to display different led effects and have hit a problem which seems to behave very unpredictably and has got me stuck. Basically I load up several buffers of rgb values for each LED and then I want to fade between them. Before I got to writing the part that actually fades between them I hit a problem, the program crashes when I try and assign the buffer to the second argument in fade (the first or the third seem to work fine). Below is the code for the fade function.

struct buf* fade(int n_args, ...){
  struct buf *buf;
  
  buf = (struct buf*) malloc(sizeof(struct buf));
  
  va_list ap;
  va_start(ap, n_args);
  

Serial.println("check");
  //buftemp = va_arg(ap, struct buf*);
  buf = va_arg(ap, struct buf*);
  buf = va_arg(ap, struct buf*);
  
  va_end(ap);
  
 return buf;
  
}

I’m also a little suspicious it might be something to do with the dynamic allocation as I’m a bit new to this and not totally convinced I’ve done it right. Below is the full sketch

#include <stdarg.h>
#include <stdio.h>
#include <avr/pgmspace.h>
#include "FastSPI_LED2.h"

#define NUM_LEDS 32
CRGB leds[NUM_LEDS];

unsigned long timings[]={
10000,
20000,
30000,
40000,
50000,
60000,
70000,
80000,
90000,
100000,
110000,
120000
};

unsigned long time = 0;
unsigned int currenteffect;
unsigned long loops;
const unsigned long lasttime=40000;
long randomstart;
const unsigned int numled = NUM_LEDS;
struct buf {
  byte colours [numled][3];
};

struct buf* fade(int, ...);
struct buf* onecolour(byte red, byte green, byte blue);
void showleds(struct buf* buf1);

void setup() {
	
  LEDS.addLeds<WS2801, 11, 13, BGR, DATA_RATE_MHZ(1)>(leds, NUM_LEDS);
  
  Serial.begin(9600);
  
}

void loop() {

  time = millis();
  struct buf *buf1, *buf2, *buf3, *buf4;
  
    while(time < timings[currenteffect]+ loops*lasttime){
    time=millis();
    buf1 = onecolour(32,0,0);
    buf2 = onecolour(0,32,0);
    buf3 = onecolour(0,0,32);
    
    buf4 = fade(3, buf1, buf3, buf2);
     delay(1);
     showleds(buf1);
     free(buf1);
     free(buf2);
     free(buf3);
     free(buf4); 
      
  }
  currenteffect++;
 }

void showleds(struct buf* buf){
  int pixel;
  for(pixel=0; pixel < numled; pixel++){
    leds[pixel]= CRGB(buf->colours[pixel][0], buf->colours[pixel][1], buf->colours[pixel][2]);
  }
  LEDS.show();
}

struct buf* onecolour(byte red, byte green, byte blue){
  int pixel;
  struct buf* buf; 
  
  buf = (struct buf*) malloc(sizeof(struct buf));
  
  for(pixel=0; pixel<numled; pixel++){
    buf->colours[pixel][0] = red;
    buf->colours[pixel][1] = green;
    buf->colours[pixel][2] = blue;  
  }  
  return buf;
}

struct buf* fade(int n_args, ...){
  struct buf *buf;
  
  buf = (struct buf*) malloc(sizeof(struct buf));
  
  va_list ap;
  va_start(ap, n_args);
  
Serial.println("check");
  //buftemp = va_arg(ap, struct buf*);
  buf = va_arg(ap, struct buf*);
  buf = va_arg(ap, struct buf*);
  
  va_end(ap);
  
 return buf;
}

Any help on this would be great as I feel like I’ve hit a brick wall on this. Oh yes I’m using a ATmega 328 on a breadboard.

Cheers

Tim

One rule when using dynamic memory: Every time you put in a malloc(), you need to have a corresponding free().

You keep allocating buf's but never free them.

Hi Keith

The call to free the memory is in the main loop just after it calls the showled function. This does mean that I'm assigning a pointer in one function and passing it back to main loop before freeing it but this all seems to worked fine without the fade function included.

On the arduino it is better to have static buffers rather than malloc'ing and free'ing all the time.

What I don't understand about your fade function is that you malloc() a buf, and then you clobber it (twice!) before you return the second argument. That seems to be a memory leak.

There is probably not a good reason to have a function with a variable number of arguments. Change your process so you don't do it.

Or, figure out the maximum number of arguments you need, and use that many.

To michinyons point, create a function with the most arguments you will use and then send a NULL for the ones you don't need.