problems with functions with variable numbers of agruments

Using Arduino Programming Questions
#1

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

#2

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.

#3

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.

#4

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.

#5

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.

#6

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.

#7