From: Daniel Elstner Date: Thu, 15 Oct 2015 16:32:44 +0000 (+0200) Subject: C++: Use smart pointers instead of manual delete X-Git-Tag: libsigrok-0.4.0~168 X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=f17b45465500f1d1aad58479802018949004cce5;p=libsigrok.git C++: Use smart pointers instead of manual delete Make use of std::unique_ptr<> to manage the lifetime of members or container elements, so that manual invocations of delete can be avoided. This also provides for exception safety. Since std::unique_ptr<> is only movable but not copyable, adapt the code to avoid copies and assignments of these pointers. --- diff --git a/bindings/cxx/classes.cpp b/bindings/cxx/classes.cpp index 7f5b3b24..5a977a86 100644 --- a/bindings/cxx/classes.cpp +++ b/bindings/cxx/classes.cpp @@ -47,9 +47,9 @@ static inline const char *valid_string(const char *input) /** Helper function to convert between map and GHashTable */ static GHashTable *map_to_hash_variant(const map &input) { - auto output = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, + auto *const output = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, reinterpret_cast(&g_variant_unref)); - for (auto entry : input) + for (const auto &entry : input) g_hash_table_insert(output, g_strdup(entry.first.c_str()), entry.second.gobj_copy()); @@ -125,21 +125,23 @@ Context::Context() : { check(sr_init(&_structure)); - struct sr_dev_driver **driver_list = sr_driver_list(_structure); - if (driver_list) - for (int i = 0; driver_list[i]; i++) - _drivers[driver_list[i]->name] = - new Driver(driver_list[i]); - const struct sr_input_module **input_list = sr_input_list(); - if (input_list) - for (int i = 0; input_list[i]; i++) - _input_formats[sr_input_id_get(input_list[i])] = - new InputFormat(input_list[i]); - const struct sr_output_module **output_list = sr_output_list(); - if (output_list) - for (int i = 0; output_list[i]; i++) - _output_formats[sr_output_id_get(output_list[i])] = - new OutputFormat(output_list[i]); + if (struct sr_dev_driver **driver_list = sr_driver_list(_structure)) + for (int i = 0; driver_list[i]; i++) { + unique_ptr driver {new Driver{driver_list[i]}}; + _drivers.emplace(driver->name(), move(driver)); + } + + if (const struct sr_input_module **input_list = sr_input_list()) + for (int i = 0; input_list[i]; i++) { + unique_ptr input {new InputFormat{input_list[i]}}; + _input_formats.emplace(input->name(), move(input)); + } + + if (const struct sr_output_module **output_list = sr_output_list()) + for (int i = 0; output_list[i]; i++) { + unique_ptr output {new OutputFormat{output_list[i]}}; + _output_formats.emplace(output->name(), move(output)); + } } string Context::package_version() @@ -155,11 +157,11 @@ string Context::lib_version() map> Context::drivers() { map> result; - for (auto entry: _drivers) + for (const auto &entry: _drivers) { - auto name = entry.first; - auto driver = entry.second; - result[name] = driver->share_owned_by(shared_from_this()); + const auto &name = entry.first; + const auto &driver = entry.second; + result.emplace(name, driver->share_owned_by(shared_from_this())); } return result; } @@ -167,11 +169,11 @@ map> Context::drivers() map> Context::input_formats() { map> result; - for (auto entry: _input_formats) + for (const auto &entry: _input_formats) { - auto name = entry.first; - auto input_format = entry.second; - result[name] = input_format->share_owned_by(shared_from_this()); + const auto &name = entry.first; + const auto &input_format = entry.second; + result.emplace(name, input_format->share_owned_by(shared_from_this())); } return result; } @@ -179,23 +181,17 @@ map> Context::input_formats() map> Context::output_formats() { map> result; - for (auto entry: _output_formats) + for (const auto &entry: _output_formats) { - auto name = entry.first; - auto output_format = entry.second; - result[name] = output_format->share_owned_by(shared_from_this()); + const auto &name = entry.first; + const auto &output_format = entry.second; + result.emplace(name, output_format->share_owned_by(shared_from_this())); } return result; } Context::~Context() { - for (auto entry : _drivers) - delete entry.second; - for (auto entry : _input_formats) - delete entry.second; - for (auto entry : _output_formats) - delete entry.second; check(sr_exit(_structure)); } @@ -283,14 +279,13 @@ shared_ptr Context::create_meta_packet( const map &config) { auto meta = g_new0(struct sr_datafeed_meta, 1); - for (auto input : config) + for (const auto &input : config) { - auto key = input.first; - auto value = input.second; - auto output = g_new(struct sr_config, 1); + const auto &key = input.first; + const auto &value = input.second; + auto *const output = g_new(struct sr_config, 1); output->key = key->id(); - output->data = value.gobj(); - g_variant_ref(output->data); + output->data = value.gobj_copy(); meta->config = g_slist_append(meta->config, output); } auto packet = g_new(struct sr_datafeed_packet, 1); @@ -322,7 +317,7 @@ shared_ptr Context::create_analog_packet( analog->meaning = meaning; - for (auto channel : channels) + for (const auto &channel : channels) meaning->channels = g_slist_append(meaning->channels, channel->_structure); analog->num_samples = num_samples; meaning->mq = static_cast(mq->id()); @@ -416,13 +411,13 @@ vector> Driver::scan( /* Translate scan options to GSList of struct sr_config pointers. */ GSList *option_list = nullptr; - for (auto entry : options) + for (const auto &entry : options) { - auto key = entry.first; - auto value = entry.second; - auto config = g_new(struct sr_config, 1); + const auto &key = entry.first; + const auto &value = entry.second; + auto *const config = g_new(struct sr_config, 1); config->key = key->id(); - config->data = value.gobj(); + config->data = const_cast(value.gobj()); option_list = g_slist_append(option_list, config); } @@ -554,24 +549,21 @@ Device::Device(struct sr_dev_inst *structure) : { for (GSList *entry = sr_dev_inst_channels_get(structure); entry; entry = entry->next) { - auto *const channel = static_cast(entry->data); - _channels[channel] = new Channel(channel); + auto *const ch = static_cast(entry->data); + unique_ptr channel {new Channel{ch}}; + _channels.emplace(ch, move(channel)); } for (GSList *entry = sr_dev_inst_channel_groups_get(structure); entry; entry = entry->next) { - auto *const group = static_cast(entry->data); - _channel_groups[group->name] = new ChannelGroup(this, group); + auto *const cg = static_cast(entry->data); + unique_ptr group {new ChannelGroup{this, cg}}; + _channel_groups.emplace(group->name(), move(group)); } } Device::~Device() -{ - for (auto entry : _channels) - delete entry.second; - for (auto entry : _channel_groups) - delete entry.second; -} +{} string Device::vendor() const { @@ -617,11 +609,11 @@ map> Device::channel_groups() { map> result; - for (auto entry: _channel_groups) + for (const auto &entry: _channel_groups) { - auto name = entry.first; - auto channel_group = entry.second; - result[name] = channel_group->share_owned_by(get_shared_from_this()); + const auto &name = entry.first; + const auto &channel_group = entry.second; + result.emplace(name, channel_group->share_owned_by(get_shared_from_this())); } return result; } @@ -679,7 +671,8 @@ shared_ptr UserDevice::add_channel(unsigned int index, index, type->id(), name.c_str())); GSList *const last = g_slist_last(sr_dev_inst_channels_get(Device::_structure)); auto *const ch = static_cast(last->data); - _channels[ch] = new Channel(ch); + unique_ptr channel {new Channel{ch}}; + _channels.emplace(ch, move(channel)); return get_channel(ch); } @@ -723,13 +716,15 @@ unsigned int Channel::index() const return _structure->index; } -ChannelGroup::ChannelGroup(Device *device, +ChannelGroup::ChannelGroup(const Device *device, struct sr_channel_group *structure) : Configurable(sr_dev_inst_driver_get(device->_structure), device->_structure, structure) { for (GSList *entry = config_channel_group->channels; entry; entry = entry->next) { auto *const ch = static_cast(entry->data); - _channels.push_back(device->_channels[ch]); + /* Note: This relies on Device::_channels to keep the Channel + * objects around over the lifetime of the ChannelGroup. */ + _channels.push_back(device->_channels.find(ch)->second.get()); } } @@ -745,7 +740,7 @@ string ChannelGroup::name() const vector> ChannelGroup::channels() { vector> result; - for (auto channel : _channels) + for (const auto &channel : _channels) result.push_back(channel->share_owned_by(_parent)); return result; } @@ -754,16 +749,15 @@ Trigger::Trigger(shared_ptr context, string name) : _structure(sr_trigger_new(name.c_str())), _context(move(context)) { - for (auto stage = _structure->stages; stage; stage = stage->next) - _stages.push_back( - new TriggerStage(static_cast(stage->data))); + for (auto *stage = _structure->stages; stage; stage = stage->next) { + unique_ptr ts {new TriggerStage{ + static_cast(stage->data)}}; + _stages.push_back(move(ts)); + } } Trigger::~Trigger() { - for (auto stage: _stages) - delete stage; - sr_trigger_free(_structure); } @@ -775,16 +769,16 @@ string Trigger::name() const vector> Trigger::stages() { vector> result; - for (auto stage : _stages) + for (const auto &stage : _stages) result.push_back(stage->share_owned_by(shared_from_this())); return result; } shared_ptr Trigger::add_stage() { - auto stage = new TriggerStage(sr_trigger_stage_add(_structure)); - _stages.push_back(stage); - return stage->share_owned_by(shared_from_this()); + unique_ptr stage {new TriggerStage{sr_trigger_stage_add(_structure)}}; + _stages.push_back(move(stage)); + return _stages.back()->share_owned_by(shared_from_this()); } TriggerStage::TriggerStage(struct sr_trigger_stage *structure) : @@ -794,8 +788,6 @@ TriggerStage::TriggerStage(struct sr_trigger_stage *structure) : TriggerStage::~TriggerStage() { - for (auto match : _matches) - delete match; } int TriggerStage::number() const @@ -806,7 +798,7 @@ int TriggerStage::number() const vector> TriggerStage::matches() { vector> result; - for (auto match : _matches) + for (const auto &match : _matches) result.push_back(match->share_owned_by(shared_from_this())); return result; } @@ -817,9 +809,10 @@ void TriggerStage::add_match(shared_ptr channel, check(sr_trigger_match_add(_structure, channel->_structure, type->id(), value)); GSList *const last = g_slist_last(_structure->matches); - _matches.push_back(new TriggerMatch( - static_cast(last->data), - move(channel))); + unique_ptr match {new TriggerMatch{ + static_cast(last->data), + move(channel)}}; + _matches.push_back(move(match)); } void TriggerStage::add_match(shared_ptr channel, @@ -901,7 +894,8 @@ Session::Session(shared_ptr context, string filename) : check(sr_session_dev_list(_structure, &dev_list)); for (GSList *dev = dev_list; dev; dev = dev->next) { auto *const sdi = static_cast(dev->data); - _owned_devices[sdi] = new SessionDevice(sdi); + unique_ptr device {new SessionDevice{sdi}}; + _owned_devices.emplace(sdi, move(device)); } _context->_session = this; } @@ -909,12 +903,6 @@ Session::Session(shared_ptr context, string filename) : Session::~Session() { check(sr_session_destroy(_structure)); - - for (auto callback : _datafeed_callbacks) - delete callback; - - for (auto entry : _owned_devices) - delete entry.second; } shared_ptr Session::get_device(const struct sr_dev_inst *sdi) @@ -1002,17 +990,16 @@ static void datafeed_callback(const struct sr_dev_inst *sdi, void Session::add_datafeed_callback(DatafeedCallbackFunction callback) { - auto cb_data = new DatafeedCallbackData(this, move(callback)); + unique_ptr cb_data + {new DatafeedCallbackData{this, move(callback)}}; check(sr_session_datafeed_callback_add(_structure, - datafeed_callback, cb_data)); - _datafeed_callbacks.push_back(cb_data); + &datafeed_callback, cb_data.get())); + _datafeed_callbacks.push_back(move(cb_data)); } void Session::remove_datafeed_callbacks() { check(sr_session_datafeed_callback_remove_all(_structure)); - for (auto callback : _datafeed_callbacks) - delete callback; _datafeed_callbacks.clear(); } @@ -1049,35 +1036,30 @@ Packet::Packet(shared_ptr device, switch (structure->type) { case SR_DF_HEADER: - _payload = new Header( + _payload.reset(new Header{ static_cast( - structure->payload)); + structure->payload)}); break; case SR_DF_META: - _payload = new Meta( + _payload.reset(new Meta{ static_cast( - structure->payload)); + structure->payload)}); break; case SR_DF_LOGIC: - _payload = new Logic( + _payload.reset(new Logic{ static_cast( - structure->payload)); + structure->payload)}); break; case SR_DF_ANALOG: - _payload = new Analog( + _payload.reset(new Analog{ static_cast( - structure->payload)); - break; - default: - _payload = nullptr; + structure->payload)}); break; } } Packet::~Packet() { - if (_payload) - delete _payload; } const PacketType *Packet::type() const @@ -1291,8 +1273,7 @@ shared_ptr InputFormat::create_input( Input::Input(shared_ptr context, const struct sr_input *structure) : _structure(structure), - _context(move(context)), - _device(nullptr) + _context(move(context)) { } @@ -1303,7 +1284,7 @@ shared_ptr Input::device() auto sdi = sr_input_dev_inst_get(_structure); if (!sdi) throw Error(SR_ERR_NA); - _device = new InputDevice(shared_from_this(), sdi); + _device.reset(new InputDevice{shared_from_this(), sdi}); } return _device->share_owned_by(shared_from_this()); @@ -1324,8 +1305,6 @@ void Input::end() Input::~Input() { - if (_device) - delete _device; sr_input_free(_structure); } diff --git a/bindings/cxx/include/libsigrokcxx/libsigrokcxx.hpp b/bindings/cxx/include/libsigrokcxx/libsigrokcxx.hpp index c7995b8c..5e101fc8 100644 --- a/bindings/cxx/include/libsigrokcxx/libsigrokcxx.hpp +++ b/bindings/cxx/include/libsigrokcxx/libsigrokcxx.hpp @@ -300,9 +300,9 @@ public: map serials(shared_ptr driver) const; private: struct sr_context *_structure; - map _drivers; - map _input_formats; - map _output_formats; + map > _drivers; + map > _input_formats; + map > _output_formats; Session *_session; LogCallbackFunction _log_callback; Context(); @@ -370,6 +370,7 @@ private: friend class Context; friend class HardwareDevice; friend class ChannelGroup; + friend class std::default_delete; }; /** A generic device, either hardware or virtual */ @@ -401,9 +402,9 @@ protected: shared_ptr get_channel(struct sr_channel *ptr); struct sr_dev_inst *_structure; - map _channels; + map > _channels; private: - map _channel_groups; + map > _channel_groups; /** Deleter needed to allow shared_ptr use with protected destructor. */ class Deleter { @@ -494,6 +495,7 @@ private: friend class Session; friend class TriggerStage; friend class Context; + friend class std::default_delete; }; /** A group of channels on a device, which share some configuration */ @@ -507,10 +509,11 @@ public: /** List of the channels in this group. */ vector > channels(); private: - ChannelGroup(Device *device, struct sr_channel_group *structure); + ChannelGroup(const Device *device, struct sr_channel_group *structure); ~ChannelGroup(); vector _channels; friend class Device; + friend class std::default_delete; }; /** A trigger configuration */ @@ -528,7 +531,7 @@ private: ~Trigger(); struct sr_trigger *_structure; shared_ptr _context; - vector _stages; + vector > _stages; friend class Deleter; friend class Context; friend class Session; @@ -554,10 +557,11 @@ public: void add_match(shared_ptr channel, const TriggerMatchType *type, float value); private: struct sr_trigger_stage *_structure; - vector _matches; + vector > _matches; explicit TriggerStage(struct sr_trigger_stage *structure); ~TriggerStage(); friend class Trigger; + friend class std::default_delete; }; /** A match condition in a trigger configuration */ @@ -577,6 +581,7 @@ private: struct sr_trigger_match *_structure; shared_ptr _channel; friend class TriggerStage; + friend class std::default_delete; }; /** Type of session stopped callback */ @@ -617,6 +622,7 @@ private: }; friend class Deleter; friend class Session; + friend class std::default_delete; }; /** A sigrok session */ @@ -661,9 +667,9 @@ private: shared_ptr get_device(const struct sr_dev_inst *sdi); struct sr_session *_structure; const shared_ptr _context; - map _owned_devices; + map > _owned_devices; map > _other_devices; - vector _datafeed_callbacks; + vector > _datafeed_callbacks; SessionStoppedCallback _stopped_callback; string _filename; shared_ptr _trigger; @@ -687,7 +693,7 @@ private: ~Packet(); const struct sr_datafeed_packet *_structure; shared_ptr _device; - PacketPayload *_payload; + unique_ptr _payload; friend class Deleter; friend class Session; friend class Output; @@ -717,6 +723,7 @@ private: friend class Deleter; friend class Packet; friend class Output; + friend class std::default_delete; }; /** Payload of a datafeed header packet */ @@ -833,6 +840,7 @@ private: friend class Context; friend class InputDevice; + friend class std::default_delete; }; /** An input instance (an input format applied to a file or stream) */ @@ -852,7 +860,7 @@ private: ~Input(); const struct sr_input *_structure; shared_ptr _context; - InputDevice *_device; + unique_ptr _device; friend class Deleter; friend class Context; friend class InputFormat; @@ -869,6 +877,7 @@ private: shared_ptr get_shared_from_this(); shared_ptr _input; friend class Input; + friend class std::default_delete; }; /** An option used by an output format */ @@ -938,6 +947,7 @@ private: friend class Context; friend class Output; + friend class std::default_delete; }; /** An output instance (an output format applied to a device) */