diff --git a/src/ItemInput.h b/src/ItemInput.h index c2662edd..70be68fd 100644 --- a/src/ItemInput.h +++ b/src/ItemInput.h @@ -29,6 +29,7 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { * @brief String value of item. */ char* value; + bool ownsValue; /** * @brief The index of first visible character. * @@ -67,6 +68,33 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { */ fptrStr callback; + char* cloneValue(const char* src) const { + const char* source = src == NULL ? "" : src; + size_t length = strlen(source); + char* clonedValue = new char[length + 1]; + memcpy(clonedValue, source, length); + clonedValue[length] = '\0'; + return clonedValue; + } + + bool setOwnedValue(const char* newValue) { + char* clonedValue = cloneValue(newValue); + if (ownsValue && value != NULL) { + delete[] value; + } + value = clonedValue; + ownsValue = true; + return true; + } + + bool ensureOwnedValue() { + if (ownsValue) { + return false; + } + setOwnedValue(value); + return true; + } + inline GraphicalValueSelectionRenderer* getGraphicalValueSelectionRenderer(MenuRenderer* renderer) const { if (renderer == NULL) { return NULL; @@ -84,7 +112,11 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { * the input is submitted. */ ItemInput(const char* text, char* value, fptrStr callback) - : MenuItem(text), value(value), callback(callback) {} + : MenuItem(text), value(value), ownsValue(false), callback(callback) { + if (value == NULL) { + setOwnedValue(""); + } + } /** * Construct a new ItemInput object with no initial value. * @@ -93,7 +125,15 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { * the input is submitted. */ ItemInput(const char* text, fptrStr callback) - : ItemInput(text, (char*)"", callback) {} + : MenuItem(text), value(NULL), ownsValue(false), callback(callback) { + setOwnedValue(""); + } + + ~ItemInput() { + if (ownsValue && value != NULL) { + delete[] value; + } + } /** * Get the current input value for this item. * @@ -111,7 +151,16 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { */ bool setValue(char* value) { if (this->value != value) { + if (ownsValue && this->value != NULL) { + delete[] this->value; + } + this->value = value; + ownsValue = false; + if (this->value == NULL) { + setOwnedValue(""); + } + LOG(F("ItemInput::setValue"), value); return true; } @@ -342,6 +391,7 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { if (strlen(value) == 0 || cursor == 0) { return; } + ensureOwnedValue(); remove(value, cursor - 1, 1); cursor--; @@ -387,8 +437,11 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { } else { concat(value, character, buf); } - delete[] value; + if (ownsValue && value != NULL) { + delete[] value; + } value = buf; + ownsValue = true; cursor++; if (getGraphicalValueSelectionRenderer(renderer) != NULL) { @@ -413,6 +466,7 @@ class ItemInput : public MenuItem, public GraphicalMenuItem { * @brief Clear the value of the input field */ void clear(MenuRenderer* renderer) { + ensureOwnedValue(); value[0] = '\0'; cursor = 0; view = 0; diff --git a/src/ItemInputCharset.h b/src/ItemInputCharset.h index d2e189e9..c9e4f0d8 100644 --- a/src/ItemInputCharset.h +++ b/src/ItemInputCharset.h @@ -143,11 +143,16 @@ class ItemInputCharset : public ItemInput { void commitCharEdit(MenuRenderer* renderer) { uint8_t length = strlen(value); if (cursor < length) { + ensureOwnedValue(); value[cursor] = charset[charsetPosition]; } else { char* buf = new char[length + 2]; concat(value, charset[charsetPosition], buf); + if (ownsValue && value != NULL) { + delete[] value; + } value = buf; + ownsValue = true; } abortCharEdit(renderer); LOG(F("ItemInputCharset::commitCharEdit"), charset[charsetPosition]); @@ -182,10 +187,16 @@ class ItemInputCharset : public ItemInput { uint8_t length = strlen(value); if (cursor < length) { - char original = value[cursor]; - value[cursor] = charset[charsetPosition]; + char* preview = new char[length + 1]; + memcpy(preview, value, length + 1); + preview[cursor] = charset[charsetPosition]; + + char* originalValue = value; + value = preview; ItemInput::draw(renderer); - value[cursor] = original; + value = originalValue; + + delete[] preview; } else { char* preview = new char[length + 2]; memcpy(preview, value, length); diff --git a/test/LcdMenu.cpp b/test/LcdMenu.cpp index 00b443f2..2035724c 100644 --- a/test/LcdMenu.cpp +++ b/test/LcdMenu.cpp @@ -278,6 +278,61 @@ unittest(clear_command_empties_input_and_resets_cursor) { assertTrue(MenuItem::isEditing()); } +unittest(default_empty_input_types_first_char_safely) { + StubRenderer renderer; + LcdMenu menu(renderer); + ItemInput item("Name", NULL); + + assertTrue(item.ownsValue); + assertTrue(item.process(&menu, ENTER)); + assertTrue(item.process(&menu, 'A')); + + assertEqual("A", item.getValue()); + assertTrue(item.ownsValue); +} + +unittest(set_value_static_empty_types_safely) { + StubRenderer renderer; + LcdMenu menu(renderer); + ItemInput item("Name", NULL); + + item.setValue((char*)""); + assertFalse(item.ownsValue); + + assertTrue(item.process(&menu, ENTER)); + assertTrue(item.process(&menu, 'B')); + + assertEqual("B", item.getValue()); + assertTrue(item.ownsValue); +} + +unittest(stack_initial_value_types_safely) { + StubRenderer renderer; + LcdMenu menu(renderer); + char stackValue[] = "X"; + ItemInput item("Name", stackValue, NULL); + + assertFalse(item.ownsValue); + assertTrue(item.process(&menu, ENTER)); + assertTrue(item.process(&menu, 'Y')); + + assertEqual("XY", item.getValue()); + assertTrue(item.ownsValue); + assertEqual("X", stackValue); +} + +unittest(clear_on_default_empty_input_is_safe) { + StubRenderer renderer; + LcdMenu menu(renderer); + ItemInput item("Name", NULL); + + assertTrue(item.process(&menu, ENTER)); + assertTrue(item.process(&menu, CLEAR)); + + assertEqual("", item.getValue()); + assertTrue(item.ownsValue); +} + unittest(hide_disables_and_clears_display) { TrackingRenderer renderer; LcdMenu menu(renderer);