From patchwork Tue Jan 1 21:23:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 118 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EB6F60B31 for ; Tue, 1 Jan 2019 22:22:33 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BA64505 for ; Tue, 1 Jan 2019 22:22:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1546377752; bh=nfgdTWa8yY4Udqvliy/qVwhEXGhtZ3ngjvmQJv3xGSU=; h=From:To:Subject:Date:From; b=mtUnx3Dm4lxqfIV5EwC18lI5f0zIpQvjEA7GQXEUgVsuDOi64bWIYXq6ao93iz05B hlXQomqXZWdLg/43NKLpRMxTh7dN7AaOLRFmiZ/5MPHEq5hOxR5eH6LK0FbSfbMIHc Vl680hRrr9oKZl29M4UBWS3hSZ616u5OciwUF108= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 1 Jan 2019 23:23:25 +0200 Message-Id: <20190101212328.18361-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/4] libcamera: mediadevice: Fix graph parsing error handling X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Jan 2019 21:22:34 -0000 Most errors recorded during graph parsing are logged but not propagated to the caller. Fix this and delete objects that are created but not successfully added to the graph to avoid memory leaks. As the error code returned from the addObject() and populate*() functions doesn't matter much, turn them into bool functions. Additionally, add a way to query whether the media graph was valid, and clear objects before populating the graph to avoid leaking them. Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham --- src/libcamera/include/media_device.h | 10 ++-- src/libcamera/media_device.cpp | 75 +++++++++++++++++----------- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/src/libcamera/include/media_device.h b/src/libcamera/include/media_device.h index bca7c9a19471..74a67c81398a 100644 --- a/src/libcamera/include/media_device.h +++ b/src/libcamera/include/media_device.h @@ -28,6 +28,7 @@ public: void close(); int populate(); + bool valid() const { return valid_; } const std::string driver() const { return driver_; } const std::string devnode() const { return devnode_; } @@ -37,18 +38,19 @@ private: std::string driver_; std::string devnode_; int fd_; + bool valid_; std::map objects_; MediaObject *object(unsigned int id); - int addObject(MediaObject *obj); + bool addObject(MediaObject *obj); void clear(); std::vector entities_; MediaEntity *getEntityByName(const std::string &name); - void populateEntities(const struct media_v2_topology &topology); - int populatePads(const struct media_v2_topology &topology); - int populateLinks(const struct media_v2_topology &topology); + bool populateEntities(const struct media_v2_topology &topology); + bool populatePads(const struct media_v2_topology &topology); + bool populateLinks(const struct media_v2_topology &topology); }; } /* namespace libcamera */ diff --git a/src/libcamera/media_device.cpp b/src/libcamera/media_device.cpp index d9a5196ac28c..fd5a31746075 100644 --- a/src/libcamera/media_device.cpp +++ b/src/libcamera/media_device.cpp @@ -56,7 +56,7 @@ namespace libcamera { * \param devnode The media device node path */ MediaDevice::MediaDevice(const std::string &devnode) - : devnode_(devnode), fd_(-1) + : devnode_(devnode), fd_(-1), valid_(false) { } @@ -99,6 +99,7 @@ void MediaDevice::clear() objects_.clear(); entities_.clear(); + valid_ = false; } /** @@ -163,18 +164,18 @@ void MediaDevice::close() * Add a new object to the global objects pool and fail if the object * has already been registered. */ -int MediaDevice::addObject(MediaObject *obj) +bool MediaDevice::addObject(MediaObject *obj) { if (objects_.find(obj->id()) != objects_.end()) { LOG(Error) << "Element with id " << obj->id() << " already enumerated."; - return -EEXIST; + return false; } objects_[obj->id()] = obj; - return 0; + return true; } /* @@ -201,7 +202,7 @@ MediaEntity *MediaDevice::getEntityByName(const std::string &name) return nullptr; } -int MediaDevice::populateLinks(const struct media_v2_topology &topology) +bool MediaDevice::populateLinks(const struct media_v2_topology &topology) { media_v2_link *mediaLinks = reinterpret_cast (topology.ptr_links); @@ -222,7 +223,7 @@ int MediaDevice::populateLinks(const struct media_v2_topology &topology) if (!source) { LOG(Error) << "Failed to find pad with id: " << source_id; - return -ENODEV; + return false; } unsigned int sink_id = mediaLinks[i].sink_id; @@ -231,20 +232,23 @@ int MediaDevice::populateLinks(const struct media_v2_topology &topology) if (!sink) { LOG(Error) << "Failed to find pad with id: " << sink_id; - return -ENODEV; + return false; } MediaLink *link = new MediaLink(&mediaLinks[i], source, sink); + if (!addObject(link)) { + delete link; + return false; + } + source->addLink(link); sink->addLink(link); - - addObject(link); } - return 0; + return true; } -int MediaDevice::populatePads(const struct media_v2_topology &topology) +bool MediaDevice::populatePads(const struct media_v2_topology &topology) { media_v2_pad *mediaPads = reinterpret_cast (topology.ptr_pads); @@ -258,16 +262,19 @@ int MediaDevice::populatePads(const struct media_v2_topology &topology) if (!mediaEntity) { LOG(Error) << "Failed to find entity with id: " << entity_id; - return -ENODEV; + return false; } MediaPad *pad = new MediaPad(&mediaPads[i], mediaEntity); - mediaEntity->addPad(pad); + if (!addObject(pad)) { + delete pad; + return false; + } - addObject(pad); + mediaEntity->addPad(pad); } - return 0; + return true; } /* @@ -275,17 +282,22 @@ int MediaDevice::populatePads(const struct media_v2_topology &topology) * reference in the MediaObject global pool and in the global vector of * entities. */ -void MediaDevice::populateEntities(const struct media_v2_topology &topology) +bool MediaDevice::populateEntities(const struct media_v2_topology &topology) { media_v2_entity *mediaEntities = reinterpret_cast (topology.ptr_entities); for (unsigned int i = 0; i < topology.num_entities; ++i) { MediaEntity *entity = new MediaEntity(&mediaEntities[i]); + if (!addObject(entity)) { + delete entity; + return false; + } - addObject(entity); entities_.push_back(entity); } + + return true; } /** @@ -311,6 +323,8 @@ int MediaDevice::populate() __u64 version = -1; int ret; + clear(); + /* * Keep calling G_TOPOLOGY until the version number stays stable. */ @@ -343,24 +357,29 @@ int MediaDevice::populate() } /* Populate entities, pads and links. */ - populateEntities(topology); - - ret = populatePads(topology); - if (ret) - goto error_free_objs; - - ret = populateLinks(topology); -error_free_objs: - if (ret) - clear(); + if (populateEntities(topology) && + populatePads(topology) && + populateLinks(topology)) + valid_ = true; delete[] links; delete[] ents; delete[] pads; - return ret; + if (!valid_) { + clear(); + return -EINVAL; + } + + return 0; } +/** + * \fn MediaDevice::valid() + * \brief Query whether the media graph is valid + * \return true if the media graph is valid, false otherwise + */ + /** * \var MediaDevice::objects_ * \brief Global map of media objects (entities, pads, links) keyed by their