Estoy luchando con este problema que parece que puedo solucionar. Al definir una clase vectorial, parezco para obtener algunos problemas al eliminar el puntero asignado.
Curiosamente, esto solo sucede después de que «leo» la memoria. Lo que quiero decir con esto es que si empujo una serie de valores al vector, parece que se quedan allí. Pero una vez que leo los valores, el puntero de datos parece volverse inválido de algún tipo, por lo que me bloqueo al intentar desasignarlo.
Código:
#ifndef VECTOR_H #define VECTOR_H #include <string.h> /* memcpy */ #define min(a,b) (a < b) ? a : b template <typename T> class vector { public: vector() : __capacity(0), __count(0), __data(0) {} //empty constructor vector(T obj, int count = 1) { if (this->__data) { // free data ptr if used delete this->__data; } this->__data = new T[count]; this->__count = count; this->__capacity = count; for (int i = 0; i < count; i++) //populate array with given object this->__data[i] = obj; } ~vector() { if (this->__data) { // free data ptr if used delete [] this->__data; } this->__count = this->__capacity = 0; } T const & operator[] (unsigned int idx) const { if (idx < this->__count) { return this->__data[idx]; } else { // throw exception or handle error to be implemented } } T& operator[] (unsigned int idx) { if (idx < this->__count) { return this->__data[idx]; } else { // throw exception or handle error to be implemented } } void resize_to_fit() { resize(this->__count); } T& pop_back(){ return this->__data[--this->__count]; } void push_back(T obj) { if (this->__capacity == this->__count) { resize(this->__capacity + 1); } this->__data[this->__count++] = obj; } bool isempty() { return !this->__data || !this->capacity || !this->size; } void clear() { this->~vector(); } T* data() { return this->__data; } int size() { return this->__count; } int capacity() { return this->__capacity; } private: void resize(int capacity) { if (this->__data == nullptr) { this->__data = new T[capacity]; this->__count = 0; this->__capacity = capacity; } else if (capacity != this->__capacity) { //else do nothing T* data = new T[capacity]; this->__count = min(this->__count, capacity); this->__capacity = capacity; memcpy(data, this->__data, sizeof(T) * this->__count); delete this->__data; //program crashes here, but the pointer is already broken... this->__data = data; } } protected: int __capacity; int __count; T* __data; }; #endif//VECTOR_H
Estaba usando este código en mi Arduino, no estoy seguro si ayuda
void print_vec(vector<int> v){ Serial.println("Printing new vec!"); for( int i = 0; i < v.size(); i++){ Serial.println(v[i]); } } void setup() { // put your setup code here, to run once: Serial.begin(9600); // Open serial connection to report values to host while(!Serial); //Waiting for serial to open vector<int> i = vector<int>(); i.push_back(10); i.push_back(2); print_vec(i); //prints 10 and 2, perfect so far i.push_back(3); while(true){ print_vec(i); i.pop_back(); delay(2000); } } void loop() { // put your main code here, to run repeatedly: }
Este código da como resultado: ¡Imprimiendo nuevo vec! 10 2 Imprimiendo nuevo vec! 0 2 3 Imprimiendo nuevo vec! 0 2 Imprimiendo nuevo vec! 0 (…)
¿Qué podría estar causando esto? Estoy bloqueado por un tiempo, se agradece cualquier idea sobre cómo resolver esto. ¡Gracias!
Comentarios
Respuesta
Tuviste muchos problemas con tu código, así que es difícil saber por dónde empezar. 🙂
Estoy de acuerdo con lo que dijo John Burger.
Llevé su código a mi PC para ahorrarme el tiempo de cargarlo cada vez, y también para poder usar valgrind en él. Ciertamente valgrind informó un error después de imprimir el vector. La razón de esto es simple. Pasó una copia de la clase a print_vec
, lo que significa que el destructor obtuvo llamado cuando print_vec
sale, desasignando así la memoria. Debería haber tenido un constructor de copias . Sin él, el compilador hace una copia bit a bit de la clase, lo que significa que ahora tiene dos objetos que comparten la misma memoria asignada.
Una solución rápida y sucia es llamar a print_vec
por referencia:
void print_vec(vector<int> & v){
Sin embargo, eso deja el error al acecho para el futuro.
Implementé el constructor de copia en mi ejemplo a continuación, sin embargo, llamar a print_vec
por referencia evita que la clase tenga que ser copiada, lo que reduce la cantidad de new/delete
que está haciendo, lo que posiblemente reduce fragmentación de la memoria.
Como dijo John Burger: ¡no llame al destructor usted mismo! Usted no puedo hacer eso. Si quieres hacer lo mismo en el destructor y la función clear
, simplemente haz que el destructor llame a clear()
.
El uso de subrayados dobles iniciales en variables es contrario al estándar C ++. No haga eso. Utilice un guión bajo al final si desea indicar una variable miembro. Pierde todas las this->
referencias. Simplemente desordenan las cosas.
Ya que asigna una matriz, debe usar delete []
; lo hizo en un lugar pero no en el otro.
¿Por qué hago esto? Es una tarea innecesaria:
vector<int> i = vector<int>();
Simplemente haz:
vector<int> i;
Si vas a asignar la clase por alguna razón, también debes implementar un operator=
o tendrás los mismos problemas que tuviste con el constructor de copia. (Ver mi ejemplo).
Aquí está probando funciones (capacidad y tamaño) sin llamarlas:
bool isempty() { return !this->__data || !this->capacity || !this->size; }
Realmente no puede hacer esto :
T const & operator[] (unsigned int idx) const { if (idx < this->__count) { return this->__data[idx]; } else { // throw exception or handle error to be implemented } }
Si toma la ruta «else», la función no devuelve ningún valor (recibe una advertencia del compilador si activa las advertencias).
Mi ejemplo reelaborado, con mis sugerencias en él. Se compila sin advertencias ni errores y se ejecuta sin fallar (en una PC):
// g++ -std=c++11 -g3 test.cpp -o test // OR: clang -std=c++11 -g3 -Wall test.cpp -lstdc++ -o test // // valgrind --leak-check=yes ./test #include <stdio.h> // printf #include <string.h> /* memcpy */ #define min(a,b) (a < b) ? a : b template <typename T> class vector { public: //empty constructor vector() : capacity_(0), count_(0), data_(nullptr) {} // copy constructor vector (const vector & rhs) : capacity_(0), count_(0), data_(nullptr) { data_ = new T [rhs.capacity_]; capacity_ = rhs.capacity_; count_ = rhs.count_; memcpy (data_, rhs.data_, sizeof (T) * count_); } // end of copy constructor // operator= vector & operator= (const vector & rhs) { // gracefully handle self-assignment (eg. a = a;) if (this == &rhs ) return *this; data_ = new T [rhs.capacity_]; capacity_ = rhs.capacity_; count_ = rhs.count_; memcpy (data_, rhs.data_, sizeof (T) * count_); return *this; } // end of operator= // destructor ~vector() { clear (); } // end of destructor T const & operator[] (unsigned int idx) const { return data_[idx]; } T& operator[] (unsigned int idx) { return data_[idx]; } void resize_to_fit() { resize(count_); } T& pop_back(){ return data_[--count_]; } void push_back(T obj) { if (capacity_ == count_) { resize(capacity_ + 1); } data_[count_++] = obj; } bool isempty() { return count_ == 0; } void clear() { delete [] data_; data_ = nullptr; count_ = capacity_ = 0; } T* data() { return data_; } int size() { return count_; } int capacity() { return capacity_; } private: void resize(int capacity) { if (data_ == nullptr) { data_ = new T[capacity]; count_ = 0; capacity_ = capacity; } else if (capacity > capacity_) { //else do nothing // allocate new memory T* data = new T[capacity]; // count can"t be higher than capacity count_ = min(count_, capacity); capacity_ = capacity; // copy data across to new pointer memcpy(data, data_, sizeof(T) * count_); // delete old pointer delete [] data_; // now remember the new pointer data_ = data; } } protected: int capacity_; int count_; T* data_; }; void print_vec(vector<int> v){ printf("%s\n", "Printing new vec!"); for( int i = 0; i < v.size(); i++){ printf("%i\n", v[i]); } } int main() { vector<int> i; i.push_back(10); i.push_back(2); print_vec(i); i.push_back(3); vector<int> j; // test assignment j = i; print_vec(j); print_vec(i); while(i.size () > 0){ print_vec(i); i.pop_back(); } }
Comentarios
- ¡Muy bien, muchas gracias! Muchos de los problemas eran una simple distracción por mi parte, estaba tratando de concentrarme en solucionar los problemas de memoria que enfrentaba al principio.
Responder
En su segundo constructor comienza con el siguiente código:
if (this->__data) { // free data ptr if used delete this->__data; } // if
¡Esto es mortal! No ha inicializado __data
, por lo que podría contener cualquier valor bajo el sol. Y no hay forma de que posiblemente sea un puntero válido a los datos existentes: es un objeto nuevo sin inicializar. Devolver este puntero de basura al montón es simplemente buscar problemas.
Elimine esas líneas, o mejor aún, use una lista de inicializadores como lo hizo con el primer constructor:
vector(T obj, int count = 1) : __data(new T[count]), __count(count), __capacity(count) { for (int i = 0; i < count; i++) //populate array with given object __data[i] = obj; } // vector(T, count)
Otro problema: en su clear()
miembro que escribió:
void clear() { this->~vector(); }
Nunca, nunca , nunca llame directamente a un destructor como este. El compilador puede poner todo tipo de código en el código del destructor, ya que «sabe» que es el único que lo llamará. Por ejemplo, algunos compiladores presionan previamente una bandera para decir si delete
el objeto del montón después de hacer la destrucción. No ha hecho eso, por lo que lo anterior puede corromper todo tipo de cosas.
En su lugar, mueva el código que está en el destructor en clear()
, y luego simplemente llame a clear()
desde el destructor.
Otro, este recogió por @Nick Gammon:
bool isempty() { return !this->__data || !this->capacity || !this->size; }
Esto está probando si el miembro __data
es falso y si el capacity
y size
. No te preocupes: los dos últimos han sido … te perdiste el prefijo __
…
[Además, deténgalo con
this->
en todas partes. «Ha utilizado un__
antes de todas sus variables miembro (lo cual va en contra de la ); no es necesario insistir en el hecho de que son variables miembro: el compilador ya lo sabe.]
Comentarios
- Eliminé todo el
vector(T obj, int count = 1)
constructor y se compiló sin errores, por lo que no era ‘ el problema en este caso, pero tiene razón en que__data
no se inicializó. -
should never have compiled
– probó las funciones en lugar de llamar ellos.capacity
es una función miembro, no una variable. Lo capté cuando intenté compilarlo bajoclang
. - ¡Ji, ji! Me perdí eso, ¡y ‘ tienes razón!
- Un pequeño consejo: tu código es prácticamente ilegible. Usa un estilo de codificación mejor : google.github.io/styleguide/cppguide.html
- Tampoco veo por qué haces esto. Aunque en toda regla STL no es compatible, hay varias depuraciones bien ed implementaciones que puede encontrar con una simple búsqueda en Google. Si lo está haciendo para aprender C ++, ¡el entorno arduino es probablemente la peor forma de hacerlo!
Respuesta
Hay algo que me gustaría agregar.
Otros ya han señalado numerosos errores en el código. Y podría señalar muchos más, pero ese no es el punto.
En mi opinión, el mayor error es: ¡implementar una clase Vector para Arduino!
Incluso si funciona, es una mala idea. Tenga en cuenta que el Arduino tiene una cantidad MUY limitada de memoria. La asignación de memoria dinámica en un espacio tan limitado suena como una mala idea: estás desperdiciando memoria, estás fragmentando el montón y no ganas casi nada, excepto algunas conveniencias de codificación.
Aún no he visto un caso en el que realmente necesita asignación de memoria dinámica en un proyecto Arduino. E incluso si lo hace, hay muchas otras herramientas que puede usar (como búferes temporales asignados a la pila, asignadores de arena, asignadores de grupos y todo eso). Pero nuevamente, probablemente el código y el costo de memoria de estas soluciones «genéricas» no serán factibles en Arduino.
La mayoría de las veces está bien usar vectores, mapas y lo que quieras en plataformas como PC (trabajo como programador de juegos de PC, así que a menudo encuentro situaciones donde los extras como estos son grandes «no-no», pero para la mayoría de las aplicaciones está bien).
Pero, en una plataforma «ajustada» como Arduino, creo que deberías estar «cerca del metal» y estar en total control de lo que está sucediendo (ciclos de memoria y CPU). Idealmente, solo debería permitirse «luxuri es «como estos cuando sabes lo que está pasando» debajo «y sabes que puedes salirte con la tuya. Lo que rara vez es el caso en plataformas «estrechas» como Arduino.
Comentarios
- Tiene un punto aquí. En particular, la idea
push_back
que asigna más memoria de forma incremental (que normalmente está bien en una PC con 4 Gb de RAM) probablemente introduzca la fragmentación de la memoria. Creo que incluso las implementaciones STL normales asignan memoria vectorial (capacidad en este caso) en lotes (por ejemplo, 10 a la vez) para reducir este problema.
i.pop_back()
deben vaciar el vector.delete this->__data; //program crashes here ...
debería usardelete[]
¿verdad? Tampoco hay un código de plataforma en su clase para que pueda depurar en una PC donde hay herramientas de depuración mucho mejores disponibles.