Using dynamic_cast incorrectly

I'm trying to use a polymorphic multidimensional array of structs and I'm running into an issue.
I am doing something wrong casting or I've made a mistake in the structure design.

const uint8_t var_string_size   = 30;
const uint8_t vname_size        = 7;
const uint8_t input_buffer_size = 30;

char    input_buffer[input_buffer_size];
uint8_t input_buffer_index;

struct var {
  char vname[vname_size];
};

struct var_long:  var {
  int32_t value;
};

struct var_float: var {
  float value;
};

struct var_string:  var {
  char value[var_string_size];
};

const uint8_t var_longs_qty   = 3;
const uint8_t var_floats_qty  = 4;
const uint8_t var_strings_qty = 2;

var_long var_longs[var_longs_qty];
var_float var_floats[var_floats_qty];
var_string var_strings[var_strings_qty];

const char    var_types[][4] = {"int", "flt", "str"};
const uint8_t var_qty[]      = {var_longs_qty, var_floats_qty, var_strings_qty};
      var     *vars[]        = {var_longs, var_floats, var_strings};
      uint8_t vars_used[]    = {0, 0, 0};

uint8_t var_type = 0;

char input_char;     
bool input_available;
  
void setup() {
  clear_vars();
  Serial.begin(115200);
  reset_buffer();
  Serial.println('.');
}

void loop() {
  if (var_type < 3) {
    if (vars_used[var_type] < var_qty[var_type]) {
      Serial.print(var_type);
      Serial.print("Name: ");
      while (not input_available) ;
      strcpy(vars[var_type][vars_used[var_type]].vname, input_buffer);
      reset_buffer();
      Serial.println(vars[var_type][vars_used[var_type]].vname);
      Serial.print("Value: ");
      while (not input_available) ;
      if (var_type == 0) {
        var_long current = dynamic_cast<var_long*>(*vars[var_type][vars_used[var_type]]);
        current.value = atol(input_buffer);
        Serial.println(current.value);
      } else if (var_type == 1) {
        var_float current = dynamic_cast<var_float*>(*vars[var_type][vars_used[var_type]]);
        current.value = atof(input_buffer);
        Serial.println(current.value);
      } else {
        var_string current = dynamic_cast<var_string*>(*vars[var_type][vars_used[var_type]]);
        strcpy(current.value, input_buffer);
        Serial.println(current.value);
      }
      reset_buffer();
      Serial.println("");
    } else {
      var_type++;
    }
    
  } else {
    Serial.println("Finished");
  }
  
}

void reset_buffer() {
  input_available    = false;
  input_buffer[0]    = '\0';
  input_buffer_index = 0;
}

void clear_vars() {
  uint8_t index;
  for (index = 0; index < var_longs_qty; index++) {
    var_longs[index].vname[0] = '\0';
    var_longs[index].value    = 0;
  }
  for (index = 0; index < var_floats_qty; index++) {
    var_floats[index].vname[0] = '\0';
    var_floats[index].value    = 0;
  }
  for (index = 0; index < var_strings_qty; index++) {
    var_strings[index].vname[0] = '\0';
    var_strings[index].value[0] = '\0';
  }
  for (index = 0; index < 3; index++) vars_used[index] = 0;
}


void serialEvent() {
  while (Serial.available()) {
    if (input_buffer_index >= input_buffer_size) {
      Serial.println(F("Input buffer overrun")); 
      while (Serial.available()) Serial.read();
      input_buffer[0] = '\0';
      input_buffer_index = 0;
    } else {
      input_char = (char)Serial.read();
      if (input_char == '\n') { // Terminate buffer
        input_buffer[input_buffer_index] = '\0';
        input_available = true;
      } else { // Buffer character
        input_buffer[input_buffer_index++] = input_char;
      }
    }
  }
}

The trouble is in the loop:

      if (var_type == 0) {
        var_long current = dynamic_cast<var_long*>(*vars[var_type][vars_used[var_type]]);
        current.value = atol(input_buffer);
        Serial.println(current.value);
      } else if (var_type == 1) {
        var_float current = dynamic_cast<var_float*>(*vars[var_type][vars_used[var_type]]);
        current.value = atof(input_buffer);
        Serial.println(current.value);
      } else {
        var_string current = dynamic_cast<var_string*>(*vars[var_type][vars_used[var_type]]);
        strcpy(current.value, input_buffer);
        Serial.println(current.value);
      }

Gives me these errors:

Arduino: 1.8.15 (Windows 10), Board: "Arduino Uno"

C:\Users\anon\Documents\Arduino\junk\junk.ino: In function 'void loop()':

junk:61:52: error: no match for 'operator*' (operand type is 'var')

         var_long current = dynamic_cast<var_long*>(*vars[var_type][vars_used[var_type]]);
                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

junk:65:54: error: no match for 'operator*' (operand type is 'var')

         var_float current = dynamic_cast<var_float*>(*vars[var_type][vars_used[var_type]]);
                                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

junk:69:56: error: no match for 'operator*' (operand type is 'var')

         var_string current = dynamic_cast<var_string*>(*vars[var_type][vars_used[var_type]]);
                                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

exit status 1

no match for 'operator*' (operand type is 'var')

This report would have more information with
"Show verbose output during compilation"
option enabled in File -> Preferences.

I seem to recall reading in the forum that dynamic_cast is not enabled (in at least some Arduino cores) because of its large runtime footprint.

That being said, I also think that you're using it incorrectly. My rudimentary understanding is that dynamic_cast is used to cast a base class pointer into a derived class pointer. Runtime processing is needed for this because it must be determined if the pointer does indeed point to an object of the correct derived class.

1 Like

There are several issues with this code.

First by doing vars[0] will give you back a var pointer, e.g. var *. If you use the array subscript (operator[]) on that again you will be accessing memory w.r.t. to the size of struct var. Since the size of struct var is not the same as struct var_long (or one of the others) this will be undefined behavior.

For dynamic_casts to work your base class needs to be polymorphic, e.g. define a virtual method (see: dynamic_cast conversion - cppreference.com):

struct V
{
    virtual void f() {} // must be polymorphic to use runtime-checked dynamic_cast
};

Besides that, like gfvalvo pointed out, it looks like the -fno-rtti flag is enabled, this prohibits the use of dynamic_cast.

Edit: I would like to add that you don't have to use dynamic_cast to downcast from base to derived. A static_cast will do just fine. The problem with a static_cast is that it won't check if a given type can be downcasted to the derived type. E.g.

struct A 
{ 
    virtual void foo() {} 
};
struct B : A {};
struct C : A {};
struct D {};

int main()
{
    B obj_b;
   
    A * pointer_to_b = &obj_b; // Implicit cast (fine since were upcasting and the types can be checked at compile time)
    C * pointer_c_safe = dynamic_cast<C*>(pointer_to_b); // This would've returned a NULL since dynamic_cast checks if the object pointed to by pointer_to_b is of type C, which it isn't
    C * pointer_c = static_cast<C*>(pointer_to_b); // Will work just fine, but undefined behavior since pointer_to_b points to type B and not C

    B * pointer_b = static_cast<B*>(pointer_to_b); // This is all fine, its up to the programmer to ensure that pointer_to_b actually points to an object of type B. In this example it does, no undefined behavior!

    D * pointer_d = static_cast<D*>(pointer_to_b); // Compile error since type A cannot be casted to D
}

Maybe you can do some research into the union data type and see if that might be a useful tool to your problem. A union allows you to store multiple types into one memory location, e.g.

union my_union
{
    int by_int;
    char by_char;
};

my_union foo;
foo.by_char = 'H';

my_union bar;
bar.by_int = 12345;

You have a rather unusual definition of "work just fine".

With the cast itself is nothing wrong, the compiler won't complain and you should be able to run your program. However if pointer_c gets dereferenced, it will be undefined behavior, anything can happen after that :-).

I just ditched the multidimensional array and address each array directly. No muss, no fuss.

Should have done it in the first place.

Thought I was past it, but I'm not.

I am trying to eliminate as much redundant code as possible. This redundancy is because of the strongly typed nature of C++. I'm thinking that overloaded functions is probably the way to go in the end but it would be ideal if I could do something like this: (There are errors in this code)

const uint8_t var_longs_qty     = 3;
const uint8_t var_floats_qty    = 4;
const uint8_t var_strings_qty   = 2;
const uint8_t var_string_size   = 30;
const uint8_t vname_size        = 7;
const uint8_t input_buffer_size = 30;

struct var_long {
  char vname[vname_size];
  int32_t value;
};

struct var_float {
  char vname[vname_size];
  float value;
};

struct var_string {
  char vname[vname_size];
  char value[var_string_size];
};

      var_long   var_longs[var_longs_qty];
      var_float  var_floats[var_floats_qty];
      var_string var_strings[var_strings_qty];
const uint8_t    var_qty[]   = {var_longs_qty, var_floats_qty, var_strings_qty};
      uint8_t    vars_used[] = {0, 0, 0};
      void       *variables[] = {var_longs, var_floats, var_strings};

uint8_t set_var(uint8_t var_type, char vname[], char value[]) {
  strcpy(variables[var_type].vname, vname);
  if (var_type == 0) {
    variables[var_type].value = atol(value);
    return 1;
  } else if (var_type == 1) {
    variables[var_type].value = atof(value);
    return 1;
  } else if (var_type == 2) {
    strcpy(variables[var_type].value, value);
    return 1;
  }
  return -1;
}

void* get_var(uint8_t var_type, char vname[]) {
  return &variables[var_type].value;
}

I looked into unions as suggested but they are not memory friendly at all unless all members are the same size in bytes.

Alternatively, I'm happy to look into other suggested methods of achieving a similar result and save as much redundant code as possible.

In your case they nearly are. An int32_t and a float take 4 bytes each. And, a String only takes about 6 bytes, I believe .... not counting the dynamically-allocated c-string. So:

#include "Arduino.h"

struct MultiVariable {
  enum VarType {
    INTEGER, FLOAT, STRING
  };

  MultiVariable(VarType t, int32_t v) : type(t), intVar(v) {}
  MultiVariable(VarType t, float v) : type(t), floatVar(v) {}
  MultiVariable(VarType t, const char *v) : type(t), stringVar(v) {}

  VarType type;
  union {
    int32_t intVar;
    float floatVar;
    String stringVar;
  };

  ~MultiVariable() {
  }
};

void printVar(MultiVariable &var);

MultiVariable variables[] = {
  {MultiVariable::FLOAT, (float) 3.14159},
  {MultiVariable::INTEGER, (int32_t) 100},
  {MultiVariable::STRING, "Hello World"}
};

void setup() {
  Serial.begin(115200);
  delay(1000);

  for (MultiVariable &i : variables) {
    printVar(i);
  }

}

void loop() {
}

void printVar(MultiVariable &var) {
  switch (var.type) {
    case MultiVariable::INTEGER:
      Serial.println(var.intVar);
      break;

    case MultiVariable::FLOAT:
      Serial.println(var.floatVar, 5);
      break;

    case MultiVariable::STRING:
      Serial.println(var.stringVar);
      break;

    default:
      break;
  }
}
1 Like

Trying to avoid the use of dynamic memory as I plan on using just about every bit of it, and a couple string allocations will certainly shred it.

However, the code your provided is quite elegant and does the job albeit in a different way than I wanted to. Thank you.
Any way to point those to a preallocated char[]?
They also definitely need their names, which it looks like you eliminated.

As I told a poster in a different thread, my go-to solution for that is to throw better hardware at the problem. Hardware is CHEAP, pulling your hair out is painful.

Why do you think that? If all you do is grab the memory (when the String is created) and hold it the entire time without de-allocating or resizing, then what would cause dreaded, so-called "shredding"?

You could define the char array size in the struct. But then all instances of the struct must be that large. Or you could handle the dynamic array yourself. But, that would only save a couple of bytes over using a String.

Same choices as with the "String" variable type. Either define a one-size-fits all char array for the name or dynamically allocate just the size you need for each name.

1 Like

But if (for what ever reason) you're stuck on a low-resource AVR, then you could keep the c-strings in PROGMEM. The downside is more clumsy initialization as it's a 2-step process (assuming you want global, compile-time initialization):

#include "Arduino.h"

const char hello[] PROGMEM = "Hello World";

struct MultiVariable {
	enum VarType {
		INTEGER, FLOAT, STRING
	};

	MultiVariable(VarType t, int32_t v) : type(t), intVar(v) {
	}
	MultiVariable(VarType t, float v) : type(t), floatVar(v) {
	}
	MultiVariable(VarType t, const char *v) : type(t), stringVar(reinterpret_cast<const __FlashStringHelper*>(v)) {
	}

	VarType type;
	union {
		int32_t intVar;
		float floatVar;
		const __FlashStringHelper *stringVar;
	};
};

void printVar(MultiVariable &var);

MultiVariable variables[] = {
		{MultiVariable::FLOAT, static_cast<float>(3.14159)},
		{MultiVariable::INTEGER, static_cast<int32_t>(100)},
		{MultiVariable::STRING, hello}
};

void setup() {
	Serial.begin(115200);
	delay(1000);

	for (MultiVariable &i : variables) {
		printVar(i);
	}

}

void loop() {
}

void printVar(MultiVariable &var) {
	switch (var.type) {
		case MultiVariable::INTEGER:
			Serial.println(var.intVar);
			break;

		case MultiVariable::FLOAT:
			Serial.println(var.floatVar, 5);
			break;

		case MultiVariable::STRING:
			Serial.println(var.stringVar);
			break;

		default:
			break;
	}
}
1 Like

Your replies are complete and insightful. Thank you for your time.

It led me to make this:

#include "Arduino.h"


const uint16_t qty_var_ints    = 20;
const uint16_t qty_var_floats  = 10;
const uint16_t qty_var_strings = 5;
const uint16_t qty_all_vars    = qty_var_ints + qty_var_floats + qty_var_strings;

int32_t var_ints[qty_var_ints];
float   var_floats[qty_var_floats];
char    var_strings[qty_var_strings][30];
char    var_names[qty_all_vars][7];

struct MultiVariable {
  enum VarType {
    INTEGER, FLOAT, STRING
  };

  MultiVariable() {
    name_index = -1;
    // TODO
  }
  MultiVariable(VarType t, const char n[], int32_t v) : type(t) {
    // TODO
  }
  MultiVariable(VarType t, const char n[], float v) : type(t) {
    // TODO
  }
  MultiVariable(VarType t, const char n[], const char v[]) : type(t) {
    // TODO
  }

  VarType type;
  int16_t name_index;
  int16_t value_index;
};

void printVar(MultiVariable &var);

MultiVariable variables[qty_all_vars];

void setup() {
  Serial.begin(115200);
  delay(1000);
  variables[0] = MultiVariable(MultiVariable::FLOAT, "pi", static_cast<float>(3.14159));
  variables[1] = MultiVariable(MultiVariable::INTEGER, "boil", static_cast<int32_t>(100));
  variables[2] = MultiVariable(MultiVariable::STRING, "hi", "Hello World!");

  for (MultiVariable &i : variables) {
    printVar(i);
  }

}

void loop() {
}

void printVar(MultiVariable &var) {
  if (var.name_index == -1) return;
  Serial.print(var_names[var.name_index]);
  Serial.print(": ");
  switch (var.type) {
    case MultiVariable::INTEGER:
      Serial.println(var_ints[var.value_index]);
      break;

    case MultiVariable::FLOAT:
      Serial.println(var_floats[var.value_index]);
      break;

    case MultiVariable::STRING:
      Serial.println(var_strings[var.value_index]);
      break;

    default:
      break;
  }
}

Which now meets my requirements.
I believe that it's completely free of dynamic memory allocation and has a small footprint in program memory.

Thanks, but your code doesn't actually do anything ... will be interested to see how you flesh out the "TODO" stuff.

No offense, but if there's a Father of OOP (which I doubt), then he'd be rolling over in his grave. Encapsulation? NOPE, Abstraction? NOPE.

But, like I said, it doesn't do anything.

None taken. The MultiVariable structure and the associated methods and functions provide enough abstraction. The command structure of the shell that this will be a part of will provide more abstraction and will protect the data from direct access.

I'm going to complete the constructors and a few other things in the morning, then it will do something.

Still working out bugs, but something like this:

const uint16_t qty_var_ints    = 25;
const uint16_t qty_var_floats  = 15;
const uint16_t qty_var_strings = 5;
const uint16_t qty_all_vars    = qty_var_ints + qty_var_floats + qty_var_strings;

int32_t var_ints[qty_var_ints];
float   var_floats[qty_var_floats];
char    var_strings[qty_var_strings][30];
char    var_names[qty_all_vars][7];

uint16_t used_var_ints    = 0;
uint16_t used_var_floats  = 0;
uint16_t used_var_strings = 0;
uint16_t used_all_vars    = 0;

struct MultiVariable {
  enum VarType {
    INTEGER, FLOAT, STRING
  };

  MultiVariable() {
    name_index = -1;
  }
  MultiVariable(VarType t, const char n[], int32_t v) : type(t) {
    name_index  = used_all_vars;
    value_index = used_var_ints;
    strcpy(var_names[used_all_vars++], n);
    var_ints[used_var_ints++]  = v;
  }
  MultiVariable(VarType t, const char n[], float v) : type(t) {
    name_index  = used_all_vars;
    value_index = used_var_floats;
    strcpy(var_names[used_all_vars++], n);
    var_floats[used_var_floats++] = v;
  }
  MultiVariable(VarType t, const char n[], const char v[]) : type(t) {
    name_index  = used_all_vars;
    value_index = used_var_strings;
    strcpy(var_names[used_all_vars++], n);
    strcpy(var_strings[used_var_strings++], v);
  }

  VarType type;
  int16_t name_index;
  int16_t value_index;
};

MultiVariable variables[qty_all_vars];
MultiVariable current;
int32_t       var_index;

int8_t   cmp_result;
uint16_t first, middle, last;

uint16_t get_index(const char n[]) {
  first = 0;
  last = used_all_vars - 1;
  middle = (first + last) / 2;
  while(first <= last) {
    cmp_result = strcmp(var_names[variables[middle].name_index], n);
    if (cmp_result == 0) return middle;
    if (cmp_result > 0) first = middle + 1;
    else last = middle - 1;
    middle = (first + last) / 2;
  }
  return middle;
}

void set_var_index(const char n[]) {
  uint16_t index = get_index(n);
  if (strcmp(var_names[variables[index].name_index], n) == 0) 
    var_index = -1;
  else
    var_index = index;
}

void shift_variables(uint16_t from_index) {
  for (uint16_t index = used_all_vars ; index > from_index ; from_index--)
    copy_variable(variables[index], variables[index - 1]);
}

void copy_variable(MultiVariable &var_dest, MultiVariable &var_src) {
  var_dest.type = var_src.type;
  var_dest.name_index = var_src.name_index;
  var_dest.value_index = var_src.value_index;
}

void new_variable(MultiVariable::VarType t, const char n[], int32_t v) {
  if (used_var_ints < qty_var_ints) {
    set_var_index(n);
    if (var_index != -1) {
      current = MultiVariable(t, n, v);
      shift_variables(var_index);
      copy_variable(variables[var_index], current);
    }
  }
}

int8_t new_variable(MultiVariable::VarType t, const char n[], float v) {
  if (used_var_ints < qty_var_floats) {
    set_var_index(n);
    if (var_index != -1) {
      current = MultiVariable(t, n, v);
      shift_variables(var_index);
      copy_variable(variables[var_index], current);
    }
  }
}

int8_t new_variable(MultiVariable::VarType t, const char n[], const char v[]) {
  if (used_var_ints < qty_var_strings) {
    set_var_index(n);
    if (var_index != -1) {
      current = MultiVariable(t, n, v);
      shift_variables(var_index);
      copy_variable(variables[var_index], current);
    }
  }
}

//MultiVariable get_variable(char n[]) {
//  set_var_index(n);
//  if (var_index != -1) 
//    return variables[var_index];
//  else
//    return NULL;   // Issue here
//}

void setup() {
  Serial.begin(115200);
  delay(1000);
  new_variable(MultiVariable::FLOAT,   "pi",   static_cast<float>(3.14159));
  new_variable(MultiVariable::INTEGER, "boil", static_cast<int32_t>(100));
  new_variable(MultiVariable::STRING,  "hi",   "Hello World!");

  for (MultiVariable &i : variables) {
    printVar(i);
  }
}

void loop() {
  
}

void printVar(MultiVariable &var) {
  if (var.name_index == -1) return;
  Serial.print(var_names[var.name_index]);
  Serial.print(": ");
  switch (var.type) {
    case MultiVariable::INTEGER:
      Serial.println(var_ints[var.value_index]);
      break;

    case MultiVariable::FLOAT:
      Serial.println(var_floats[var.value_index]);
      break;

    case MultiVariable::STRING:
      Serial.println(var_strings[var.value_index]);
      break;

    default:
      break;
  }
}

Alternatively you can make use of one of the great features C++ has to offer, that is template metaprogramming. This is a rudimentary example, but it should (mostly) work:

inline void parse_string(const char * str, float & out) { out = atof(str); }
inline void parse_string(const char * str, int32_t & out) { out = atol(str); }

template<typename T>
void print(T & var)
{
    // You can uncomment this if you want to try it in https://coliru.stacked-crooked.com/a/0664d151a0a50b13
    // std::cout << 
    //     var.get_name() << ": " <<
    //     var.get_value() << std::endl;

    Serial.print(var.get_name());
    Serial.print(": ");
    Serial.println(var.get_value());
}

class var_base
{
public:
    char const * get_name() const { return name_; }
    void set_name(char const * name) { memcpy(name_, name, min(static_cast<size_t>(name_size), strlen(name) + 1)); }
    
private:
    static constexpr uint8_t name_size = 7;

    char name_[name_size + 1] = {}; // + 1 for null terminator
};

template<typename T>
class basic_var : 
    public var_base
{
public:
    T const & get_value() const { return value_; }
    void set_value(T const & value) { value_ = value; }
    void set_value(const char * value) { parse_string(value, value_); }
    
private:
    T value_ = {};
};

// Specialization for a string variable
struct string_t {};
template<>
class basic_var<string_t> :
    public var_base
{
public:
    char const * get_value() const { return value_; }
    void set_value(char const * value) { memcpy(value_, value, min(static_cast<size_t>(value_size), strlen(value) + 1)); }
    
private:
    static constexpr uint8_t value_size = 30;
    
    char value_[value_size + 1] = {}; // + 1 for null terminator
};

template<typename T, uint8_t quantity>
class var_pool
{
public:
    basic_var<T> * find_first(char const * name)
    {
        for (uint8_t i = 0; i < last; ++i)
            if (strcmp(pool[i].get_name(), name) == 0)
                return &pool[i];
        
        if (last < quantity) {
            basic_var<T> * var = &pool[last++];
            var->set_name(name);
            return var;
        }

        return nullptr;
    }
    
    void print_all() const
    {
        for (uint8_t i = 0; i < last; ++i)
            print(pool[i]);
    }

private:
    basic_var<T> pool[quantity] = {};
    uint8_t last = {};
};

static var_pool<float, 15> float_vars;
static var_pool<int32_t, 25> int32_vars;
static var_pool<string_t, 5> string_vars;

void setup() {
  Serial.begin(115200);

  auto * var1 = int32_vars.find_first("var1");
  auto * var2 = float_vars.find_first("var2");
  auto * var3 = float_vars.find_first("var3");
  auto * var4 = string_vars.find_first("var4");

  // Set values
  var1->set_value(199);
  var2->set_value(1238.33);
  var3->set_value("123.23"); // Or by string
  var4->set_value("world");

  // Print all variables
  int32_vars.print_all();
  float_vars.print_all();
  string_vars.print_all();
}

void loop() {
}

It looks like this consumes about 10% less memory (44% vs 55% for an Arduino uno) than the code example of the previous post. You can reduce the memory usage even further if you modify it so that the variable names are stored in progmem, that is if the variable names are known at compile time of course.

I'm starting a new thread on this topic as it's diverged too much from the original question, asked and answered.
Mod said it was not cross-posting.
I'm still poking around to fix an insert issue, before I start the new thread.
Once it's up and I explain the full application, I'd value your input.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.