[{"id":11346,"web_url":"https://patchwork.libcamera.org/comment/11346/","msgid":"<7d8b822b-0c5b-e32a-762b-10eac05cc95a@ideasonboard.com>","date":"2020-07-12T19:32:11","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 12/07/2020 15:44, Laurent Pinchart wrote:\n> Add basic support to read and write data from/to a file, along with\n> retrieving and setting the current read/write position.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/file.h | 7 ++\n> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n> 2 files changed, 116 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n> index e3e72132e336..ead94cdf29c9 100644\n> --- a/include/libcamera/internal/file.h\n> +++ b/include/libcamera/internal/file.h\n> @@ -49,6 +49,12 @@ public:\n> \tint error() const { return error_; }\n> \tssize_t size() const;\n> \n> +\toff_t pos() const { return pos_; }\n> +\toff_t seek(off_t pos);\n> +\n> +\tssize_t read(const Span &data);\n> +\tssize_t write(const Span &data);\n> +\n> \tSpan map(off_t offset = 0, ssize_t size = -1,\n> \t\t\t MapFlag flags = MapNoOption);\n> \tbool unmap(uint8_t *addr);\n> @@ -61,6 +67,7 @@ private:\n> \tstd::string name_;\n> \tint fd_;\n> \tOpenMode mode_;\n> +\toff_t pos_;\n> \n> \tint error_;\n> \tstd::map maps_;\n> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n> index c471bde3fc68..8bd109090919 100644\n> --- a/src/libcamera/file.cpp\n> +++ b/src/libcamera/file.cpp\n> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n> * before performing I/O operations.\n> */\n> File::File(const std::string &name)\n> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> {\n> }\n> \n> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n> * setFileName().\n> */\n> File::File()\n> -\t: fd_(-1), mode_(NotOpen), error_(0)\n> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> {\n> }\n> \n> @@ -202,6 +202,7 @@ void File::close()\n> \t::close(fd_);\n> \tfd_ = -1;\n> \tmode_ = NotOpen;\n> +\tpos_ = 0;\n> }\n> \n> /**\n> @@ -237,6 +238,112 @@ ssize_t File::size() const\n> \treturn st.st_size;\n> }\n> \n> +/**\n> + * \\fn off_t File::pos() const\n> + * \\brief Return current read or write position\n> + *\n> + * If the file is closed, this function returns 0.\n> + *\n> + * \\return The current read or write position\n> + */\n> +\n> +/**\n> + * \\brief Set the read or write position\n> + * \\param[in] pos The desired position\n> + * \\return The resulting offset from the beginning of the file on success, or a\n> + * negative error code otherwise\n> + */\n> +off_t File::seek(off_t pos)\n> +{\n> +\tif (!isOpen())\n> +\t\treturn -EINVAL;\n> +\n> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n> +\tif (ret < 0)\n> +\t\treturn -errno;\n> +\n> +\tpos_ = ret;\n> +\treturn ret;\n> +}\n> +\n> +/**\n> + * \\brief Read data from the file\n> + * \\param[in] data Memory to read data into\n> + *\n> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n> + * return the number of bytes read. If less data than requested is available,\n> + * the returned byte count may be smaller than the requested size. If no more\n> + * data is available, 0 is returned.\n> + *\n> + * The position of the file as returned by pos() is advanced by the number of\n> + * bytes read. If an error occurs, the position isn't modified.\n> + *\n> + * \\return The number of bytes read on success, or a negative error code\n> + * otherwise\n> + */\n> +ssize_t File::read(const Span &data)\n> +{\n> +\tif (!isOpen())\n> +\t\treturn -EINVAL;\n> +\n> +\tsize_t readBytes = 0;\n> +\tssize_t ret = 0;\n> +\n> +\t/* Retry in case of interrupted system calls. */\n> +\twhile (readBytes < data.size()) {\n> +\t\tret = ::read(fd_, data.data() + readBytes,\n> +\t\t\t data.size() - readBytes);\n> +\t\tif (ret <= 0)\n> +\t\t\tbreak;\n> +\n> +\t\treadBytes += ret;\n> +\t}\n> +\n> +\tif (ret < 0 && !readBytes)\n\nthe man page for read(2) states:\n\n\n# On error, -1 is returned, and errno is set appropriately. In this\n# case, it is left unspecified whether the file position (if any)\n# changes.\n\n\nIn the event of an error, that could thus leave pos_ invalid, but I\ndon't think we care too much about that for an internal helper which\nprobably isn't going to use pos much anyway.\n\nWe could:\n\nA) call lseek to explicitly reset pos\nB) Continue to do nothing.\n\nI think we'll choose option B for now, especially as the function\ndocumentation for both explicitly states that the pos will not be moved\non error. ;-)\n\n\nSo ...\n\nReviewed-by: Kieran Bingham \n\n\n> +\t\treturn -errno;\n> +\n> +\tpos_ += readBytes;\n> +\treturn readBytes;\n> +}\n> +\n> +/**\n> + * \\brief Write data to the file\n> + * \\param[in] data Memory containing data to be written\n> + *\n> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n> + * return the number of bytes written. If the file system doesn't have enough\n> + * space for the data, the returned byte count may be less than requested.\n> + *\n> + * The position of the file as returned by pos() is advanced by the number of\n> + * bytes written. If an error occurs, the position isn't modified.\n> + *\n> + * \\return The number of bytes written on success, or a negative error code\n> + * otherwise\n> + */\n> +ssize_t File::write(const Span &data)\n> +{\n> +\tif (!isOpen())\n> +\t\treturn -EINVAL;\n> +\n> +\tsize_t writtenBytes = 0;\n> +\n> +\t/* Retry in case of interrupted system calls. */\n> +\twhile (writtenBytes < data.size()) {\n> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n> +\t\t\t\t data.size() - writtenBytes);\n> +\t\tif (ret <= 0)\n> +\t\t\tbreak;\n> +\n> +\t\twrittenBytes += ret;\n> +\t}\n> +\n> +\tif (data.size() && !writtenBytes)\n> +\t\treturn -errno;\n> +\n> +\tpos_ += writtenBytes;> +\treturn writtenBytes;\n> +}\n> +\n> /**\n> * \\brief Map a region of the file in the process memory\n> * \\param[in] offset The region offset within the file\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3F2ECBD792\n\tfor ;\n\tSun, 12 Jul 2020 19:32:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ABE1460595;\n\tSun, 12 Jul 2020 21:32:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B54CC6055B\n\tfor ;\n\tSun, 12 Jul 2020 21:32:15 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB66124A;\n\tSun, 12 Jul 2020 21:32:14 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qNC8NIv/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594582334;\n\tbh=vu8/NMmtCFmiwphiNuamCVOyjjP1aM6hOUSitLSRqBs=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=qNC8NIv/c0NaKMx5I+kdFI1z7qNYb0iRPLbGFDCyYAyy2b3MzuW58QJVx6lgY0T6u\n\t9brUguffYVND5XUuLYAx6PE5O8TmITzJhX4LSKXmCbq8NFGVj+oubm4Be2bqSTyrzJ\n\tZVg+lp+LTl7YmsOITzvFYbQW2pTB4Po0nj2O8vAE=","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7d8b822b-0c5b-e32a-762b-10eac05cc95a@ideasonboard.com>","Date":"Sun, 12 Jul 2020 20:32:11 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11351,"web_url":"https://patchwork.libcamera.org/comment/11351/","msgid":"<20200713065349.GE1498036@oden.dyn.berto.se>","date":"2020-07-13T06:53:49","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your work.\n\nOn 2020-07-12 17:44:16 +0300, Laurent Pinchart wrote:\n> Add basic support to read and write data from/to a file, along with\n> retrieving and setting the current read/write position.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/file.h | 7 ++\n> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n> 2 files changed, 116 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n> index e3e72132e336..ead94cdf29c9 100644\n> --- a/include/libcamera/internal/file.h\n> +++ b/include/libcamera/internal/file.h\n> @@ -49,6 +49,12 @@ public:\n> \tint error() const { return error_; }\n> \tssize_t size() const;\n> \n> +\toff_t pos() const { return pos_; }\n\nWould it not be nicer to return lseek(fd_, 0, SEEK_CUR) here? That way \nwe could avoid keeping pos_ in sync in potential corner cases.\n\n> +\toff_t seek(off_t pos);\n> +\n> +\tssize_t read(const Span &data);\n> +\tssize_t write(const Span &data);\n> +\n> \tSpan map(off_t offset = 0, ssize_t size = -1,\n> \t\t\t MapFlag flags = MapNoOption);\n> \tbool unmap(uint8_t *addr);\n> @@ -61,6 +67,7 @@ private:\n> \tstd::string name_;\n> \tint fd_;\n> \tOpenMode mode_;\n> +\toff_t pos_;\n> \n> \tint error_;\n> \tstd::map maps_;\n> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n> index c471bde3fc68..8bd109090919 100644\n> --- a/src/libcamera/file.cpp\n> +++ b/src/libcamera/file.cpp\n> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n> * before performing I/O operations.\n> */\n> File::File(const std::string &name)\n> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> {\n> }\n> \n> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n> * setFileName().\n> */\n> File::File()\n> -\t: fd_(-1), mode_(NotOpen), error_(0)\n> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> {\n> }\n> \n> @@ -202,6 +202,7 @@ void File::close()\n> \t::close(fd_);\n> \tfd_ = -1;\n> \tmode_ = NotOpen;\n> +\tpos_ = 0;\n> }\n> \n> /**\n> @@ -237,6 +238,112 @@ ssize_t File::size() const\n> \treturn st.st_size;\n> }\n> \n> +/**\n> + * \\fn off_t File::pos() const\n> + * \\brief Return current read or write position\n> + *\n> + * If the file is closed, this function returns 0.\n> + *\n> + * \\return The current read or write position\n> + */\n> +\n> +/**\n> + * \\brief Set the read or write position\n> + * \\param[in] pos The desired position\n> + * \\return The resulting offset from the beginning of the file on success, or a\n> + * negative error code otherwise\n> + */\n> +off_t File::seek(off_t pos)\n> +{\n> +\tif (!isOpen())\n> +\t\treturn -EINVAL;\n> +\n> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n> +\tif (ret < 0)\n> +\t\treturn -errno;\n> +\n> +\tpos_ = ret;\n> +\treturn ret;\n> +}\n> +\n> +/**\n> + * \\brief Read data from the file\n> + * \\param[in] data Memory to read data into\n> + *\n> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n> + * return the number of bytes read. If less data than requested is available,\n> + * the returned byte count may be smaller than the requested size. If no more\n> + * data is available, 0 is returned.\n> + *\n> + * The position of the file as returned by pos() is advanced by the number of\n> + * bytes read. If an error occurs, the position isn't modified.\n> + *\n> + * \\return The number of bytes read on success, or a negative error code\n> + * otherwise\n> + */\n> +ssize_t File::read(const Span &data)\n> +{\n> +\tif (!isOpen())\n> +\t\treturn -EINVAL;\n> +\n> +\tsize_t readBytes = 0;\n> +\tssize_t ret = 0;\n> +\n> +\t/* Retry in case of interrupted system calls. */\n> +\twhile (readBytes < data.size()) {\n> +\t\tret = ::read(fd_, data.data() + readBytes,\n> +\t\t\t data.size() - readBytes);\n> +\t\tif (ret <= 0)\n> +\t\t\tbreak;\n> +\n> +\t\treadBytes += ret;\n> +\t}\n> +\n> +\tif (ret < 0 && !readBytes)\n> +\t\treturn -errno;\n> +\n> +\tpos_ += readBytes;\n> +\treturn readBytes;\n> +}\n> +\n> +/**\n> + * \\brief Write data to the file\n> + * \\param[in] data Memory containing data to be written\n> + *\n> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n> + * return the number of bytes written. If the file system doesn't have enough\n> + * space for the data, the returned byte count may be less than requested.\n> + *\n> + * The position of the file as returned by pos() is advanced by the number of\n> + * bytes written. If an error occurs, the position isn't modified.\n> + *\n> + * \\return The number of bytes written on success, or a negative error code\n> + * otherwise\n> + */\n> +ssize_t File::write(const Span &data)\n> +{\n> +\tif (!isOpen())\n> +\t\treturn -EINVAL;\n> +\n> +\tsize_t writtenBytes = 0;\n> +\n> +\t/* Retry in case of interrupted system calls. */\n> +\twhile (writtenBytes < data.size()) {\n> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n> +\t\t\t\t data.size() - writtenBytes);\n> +\t\tif (ret <= 0)\n> +\t\t\tbreak;\n> +\n> +\t\twrittenBytes += ret;\n> +\t}\n> +\n> +\tif (data.size() && !writtenBytes)\n> +\t\treturn -errno;\n> +\n> +\tpos_ += writtenBytes;\n> +\treturn writtenBytes;\n> +}\n> +\n> /**\n> * \\brief Map a region of the file in the process memory\n> * \\param[in] offset The region offset within the file\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 28A37BD790\n\tfor ;\n\tMon, 13 Jul 2020 06:53:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AD0EE605AA;\n\tMon, 13 Jul 2020 08:53:52 +0200 (CEST)","from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 652A66055B\n\tfor ;\n\tMon, 13 Jul 2020 08:53:51 +0200 (CEST)","by mail-lf1-x142.google.com with SMTP id g139so7900066lfd.10\n\tfor ;\n\tSun, 12 Jul 2020 23:53:51 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ty2sm3906633lji.8.2020.07.12.23.53.49\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 12 Jul 2020 23:53:49 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"x5P6bGDB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=Z9ipCLdJVQ5r0sQnCwpjRJ7yXVEKkr0n3JMnhOdSDio=;\n\tb=x5P6bGDB7zUsc/2LJ2T02zZ8PMTVUdX00SsvzF5KegwvLN1xn6Lfv507x5MZRv7m8R\n\tweP10NBOJqYpTx8CYf+QmXegSJiluIKAC50tNJT55dlzLEQmPRw6Ngf+ZlFnhFeB4iTt\n\t0aJ07jUsXUNSz375duNHprvjhtegm2YLK7lJf6nJpC53X1Y7SNJh1xllrMBP02nsNCsG\n\tC5SUaXNw02IwNZoW5RKsGFTUPDQyV6sbai2hu71L917zOtd9sQ7mpaWUmr19smHPpl6F\n\tQohHN4TYZoCsrS/ZkvRCn5u8NcF3d/F5drM5g4+hXcZhlbQz8JzpqiMXZodRrSDgOpmn\n\t1pSw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=Z9ipCLdJVQ5r0sQnCwpjRJ7yXVEKkr0n3JMnhOdSDio=;\n\tb=styIwgrg0hBHAq077Q2A2sbLEZflQIsUv2ttlg9db7knVhVqXFoRbx6Gd6YBEbxqAG\n\thWY4X/w4Us9BkuSH9RxzVByCz98ilJo3f2HZJZF/h+Ld3Ad4ocgwmor3SB8pEBQft0CK\n\ttSN+lkTWplj5yBHBQGOALAxIltQAw0okZV/is8WNYkMCRCrNvF2X1aAuJmLQjrFaX1lj\n\tOL7KQyS9yA+eW4Y9SwCzlE2xMYu+rHZ6kYh2J70eburSUHewdvix6yYiNfLpA5q0sGRw\n\tRCjRyQsVhkWCpEC3fV2Qfs6fLQaHrxSnFjFs4e4Nly0yfFSJmh7SEr9gJ4x6K00K78Vj\n\tUvqw==","X-Gm-Message-State":"AOAM531K8HCRcVGh02C58gKO00/WB4OPfGnBU2Z+HqxhP8W2Vv1IxwJL\n\t3y2ApsawlddZ6qwOnWVbQEjebg==","X-Google-Smtp-Source":"ABdhPJzlY95gNZkRxXI1OD35V4yDlBYhOlVhdIJmairi2NESpfEy4eAXFPxGyYU5Xx6W1CM5k5Z2lQ==","X-Received":"by 2002:a19:911:: with SMTP id 17mr30291718lfj.80.1594623230499; \n\tSun, 12 Jul 2020 23:53:50 -0700 (PDT)","Date":"Mon, 13 Jul 2020 08:53:49 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200713065349.GE1498036@oden.dyn.berto.se>","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11355,"web_url":"https://patchwork.libcamera.org/comment/11355/","msgid":"<7532dd8a-a002-7813-8fbc-ee7dc34a0e8f@ideasonboard.com>","date":"2020-07-13T08:31:42","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas / Laurent,\n\nOn 13/07/2020 07:53, Niklas Söderlund wrote:\n> Hi Laurent,\n> \n> Thanks for your work.\n> \n> On 2020-07-12 17:44:16 +0300, Laurent Pinchart wrote:\n>> Add basic support to read and write data from/to a file, along with\n>> retrieving and setting the current read/write position.\n>>\n>> Signed-off-by: Laurent Pinchart \n>> ---\n>> include/libcamera/internal/file.h | 7 ++\n>> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n>> 2 files changed, 116 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n>> index e3e72132e336..ead94cdf29c9 100644\n>> --- a/include/libcamera/internal/file.h\n>> +++ b/include/libcamera/internal/file.h\n>> @@ -49,6 +49,12 @@ public:\n>> \tint error() const { return error_; }\n>> \tssize_t size() const;\n>> \n>> +\toff_t pos() const { return pos_; }\n> \n> Would it not be nicer to return lseek(fd_, 0, SEEK_CUR) here? That way \n> we could avoid keeping pos_ in sync in potential corner cases.\n\nAha ... this would of course address my concerns about pos_ becoming\ninvalid on error cases ...\n\n--\nKieran\n\n\n> \n>> +\toff_t seek(off_t pos);\n>> +\n>> +\tssize_t read(const Span &data);\n>> +\tssize_t write(const Span &data);\n>> +\n>> \tSpan map(off_t offset = 0, ssize_t size = -1,\n>> \t\t\t MapFlag flags = MapNoOption);\n>> \tbool unmap(uint8_t *addr);\n>> @@ -61,6 +67,7 @@ private:\n>> \tstd::string name_;\n>> \tint fd_;\n>> \tOpenMode mode_;\n>> +\toff_t pos_;\n>> \n>> \tint error_;\n>> \tstd::map maps_;\n>> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n>> index c471bde3fc68..8bd109090919 100644\n>> --- a/src/libcamera/file.cpp\n>> +++ b/src/libcamera/file.cpp\n>> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n>> * before performing I/O operations.\n>> */\n>> File::File(const std::string &name)\n>> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n>> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n>> {\n>> }\n>> \n>> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n>> * setFileName().\n>> */\n>> File::File()\n>> -\t: fd_(-1), mode_(NotOpen), error_(0)\n>> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n>> {\n>> }\n>> \n>> @@ -202,6 +202,7 @@ void File::close()\n>> \t::close(fd_);\n>> \tfd_ = -1;\n>> \tmode_ = NotOpen;\n>> +\tpos_ = 0;\n>> }\n>> \n>> /**\n>> @@ -237,6 +238,112 @@ ssize_t File::size() const\n>> \treturn st.st_size;\n>> }\n>> \n>> +/**\n>> + * \\fn off_t File::pos() const\n>> + * \\brief Return current read or write position\n>> + *\n>> + * If the file is closed, this function returns 0.\n>> + *\n>> + * \\return The current read or write position\n>> + */\n>> +\n>> +/**\n>> + * \\brief Set the read or write position\n>> + * \\param[in] pos The desired position\n>> + * \\return The resulting offset from the beginning of the file on success, or a\n>> + * negative error code otherwise\n>> + */\n>> +off_t File::seek(off_t pos)\n>> +{\n>> +\tif (!isOpen())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n>> +\tif (ret < 0)\n>> +\t\treturn -errno;\n>> +\n>> +\tpos_ = ret;\n>> +\treturn ret;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Read data from the file\n>> + * \\param[in] data Memory to read data into\n>> + *\n>> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n>> + * return the number of bytes read. If less data than requested is available,\n>> + * the returned byte count may be smaller than the requested size. If no more\n>> + * data is available, 0 is returned.\n>> + *\n>> + * The position of the file as returned by pos() is advanced by the number of\n>> + * bytes read. If an error occurs, the position isn't modified.\n>> + *\n>> + * \\return The number of bytes read on success, or a negative error code\n>> + * otherwise\n>> + */\n>> +ssize_t File::read(const Span &data)\n>> +{\n>> +\tif (!isOpen())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tsize_t readBytes = 0;\n>> +\tssize_t ret = 0;\n>> +\n>> +\t/* Retry in case of interrupted system calls. */\n>> +\twhile (readBytes < data.size()) {\n>> +\t\tret = ::read(fd_, data.data() + readBytes,\n>> +\t\t\t data.size() - readBytes);\n>> +\t\tif (ret <= 0)\n>> +\t\t\tbreak;\n>> +\n>> +\t\treadBytes += ret;\n>> +\t}\n>> +\n>> +\tif (ret < 0 && !readBytes)\n>> +\t\treturn -errno;\n>> +\n>> +\tpos_ += readBytes;\n>> +\treturn readBytes;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Write data to the file\n>> + * \\param[in] data Memory containing data to be written\n>> + *\n>> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n>> + * return the number of bytes written. If the file system doesn't have enough\n>> + * space for the data, the returned byte count may be less than requested.\n>> + *\n>> + * The position of the file as returned by pos() is advanced by the number of\n>> + * bytes written. If an error occurs, the position isn't modified.\n>> + *\n>> + * \\return The number of bytes written on success, or a negative error code\n>> + * otherwise\n>> + */\n>> +ssize_t File::write(const Span &data)\n>> +{\n>> +\tif (!isOpen())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tsize_t writtenBytes = 0;\n>> +\n>> +\t/* Retry in case of interrupted system calls. */\n>> +\twhile (writtenBytes < data.size()) {\n>> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n>> +\t\t\t\t data.size() - writtenBytes);\n>> +\t\tif (ret <= 0)\n>> +\t\t\tbreak;\n>> +\n>> +\t\twrittenBytes += ret;\n>> +\t}\n>> +\n>> +\tif (data.size() && !writtenBytes)\n>> +\t\treturn -errno;\n>> +\n>> +\tpos_ += writtenBytes;\n>> +\treturn writtenBytes;\n>> +}\n>> +\n>> /**\n>> * \\brief Map a region of the file in the process memory\n>> * \\param[in] offset The region offset within the file\n>> -- \n>> Regards,\n>>\n>> Laurent Pinchart\n>>\n>> _______________________________________________\n>> libcamera-devel mailing list\n>> libcamera-devel@lists.libcamera.org\n>> https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 66886BD790\n\tfor ;\n\tMon, 13 Jul 2020 08:31:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F0B42605B5;\n\tMon, 13 Jul 2020 10:31:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4CD3A605B1\n\tfor ;\n\tMon, 13 Jul 2020 10:31:46 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71109C9;\n\tMon, 13 Jul 2020 10:31:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EBNC2y2C\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594629105;\n\tbh=Efj5OQ5TaJWWAdGNKoaQc5aXQtwuRt28Gv+nJN48hlk=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=EBNC2y2CsVRIPKw51q16Oagg4Kd/Ns7MflRIvW9iO57fS8XE28j4AgSi/7geC+kN9\n\tP1J26ldbvoQ1Au7WCI/dWx9aKgeNjzil6zDn6+PRqpohPnxKfkp7y08dE22N6wvDhp\n\tlB8NMKybdCKtDgr9S89XrZ+Bq5pW//iq7SvZVQxg=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tLaurent Pinchart ","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>\n\t<20200713065349.GE1498036@oden.dyn.berto.se>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<7532dd8a-a002-7813-8fbc-ee7dc34a0e8f@ideasonboard.com>","Date":"Mon, 13 Jul 2020 09:31:42 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200713065349.GE1498036@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11356,"web_url":"https://patchwork.libcamera.org/comment/11356/","msgid":"","date":"2020-07-13T08:34:27","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 12/07/2020 20:32, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 12/07/2020 15:44, Laurent Pinchart wrote:\n>> Add basic support to read and write data from/to a file, along with\n>> retrieving and setting the current read/write position.\n>>\n>> Signed-off-by: Laurent Pinchart \n>> ---\n>> include/libcamera/internal/file.h | 7 ++\n>> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n>> 2 files changed, 116 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n>> index e3e72132e336..ead94cdf29c9 100644\n>> --- a/include/libcamera/internal/file.h\n>> +++ b/include/libcamera/internal/file.h\n>> @@ -49,6 +49,12 @@ public:\n>> \tint error() const { return error_; }\n>> \tssize_t size() const;\n>> \n>> +\toff_t pos() const { return pos_; }\n>> +\toff_t seek(off_t pos);\n>> +\n>> +\tssize_t read(const Span &data);\n>> +\tssize_t write(const Span &data);\n>> +\n>> \tSpan map(off_t offset = 0, ssize_t size = -1,\n>> \t\t\t MapFlag flags = MapNoOption);\n>> \tbool unmap(uint8_t *addr);\n>> @@ -61,6 +67,7 @@ private:\n>> \tstd::string name_;\n>> \tint fd_;\n>> \tOpenMode mode_;\n>> +\toff_t pos_;\n>> \n>> \tint error_;\n>> \tstd::map maps_;\n>> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n>> index c471bde3fc68..8bd109090919 100644\n>> --- a/src/libcamera/file.cpp\n>> +++ b/src/libcamera/file.cpp\n>> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n>> * before performing I/O operations.\n>> */\n>> File::File(const std::string &name)\n>> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n>> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n>> {\n>> }\n>> \n>> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n>> * setFileName().\n>> */\n>> File::File()\n>> -\t: fd_(-1), mode_(NotOpen), error_(0)\n>> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n>> {\n>> }\n>> \n>> @@ -202,6 +202,7 @@ void File::close()\n>> \t::close(fd_);\n>> \tfd_ = -1;\n>> \tmode_ = NotOpen;\n>> +\tpos_ = 0;\n>> }\n>> \n>> /**\n>> @@ -237,6 +238,112 @@ ssize_t File::size() const\n>> \treturn st.st_size;\n>> }\n>> \n>> +/**\n>> + * \\fn off_t File::pos() const\n>> + * \\brief Return current read or write position\n>> + *\n>> + * If the file is closed, this function returns 0.\n>> + *\n>> + * \\return The current read or write position\n>> + */\n>> +\n>> +/**\n>> + * \\brief Set the read or write position\n>> + * \\param[in] pos The desired position\n>> + * \\return The resulting offset from the beginning of the file on success, or a\n>> + * negative error code otherwise\n>> + */\n>> +off_t File::seek(off_t pos)\n>> +{\n>> +\tif (!isOpen())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n>> +\tif (ret < 0)\n>> +\t\treturn -errno;\n>> +\n>> +\tpos_ = ret;\n>> +\treturn ret;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Read data from the file\n>> + * \\param[in] data Memory to read data into\n>> + *\n>> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n>> + * return the number of bytes read. If less data than requested is available,\n>> + * the returned byte count may be smaller than the requested size. If no more\n>> + * data is available, 0 is returned.\n>> + *\n>> + * The position of the file as returned by pos() is advanced by the number of\n>> + * bytes read. If an error occurs, the position isn't modified.\n>> + *\n>> + * \\return The number of bytes read on success, or a negative error code\n>> + * otherwise\n>> + */\n>> +ssize_t File::read(const Span &data)\n>> +{\n>> +\tif (!isOpen())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tsize_t readBytes = 0;\n>> +\tssize_t ret = 0;\n>> +\n>> +\t/* Retry in case of interrupted system calls. */\n>> +\twhile (readBytes < data.size()) {\n>> +\t\tret = ::read(fd_, data.data() + readBytes,\n>> +\t\t\t data.size() - readBytes);\n>> +\t\tif (ret <= 0)\n>> +\t\t\tbreak;\n>> +\n>> +\t\treadBytes += ret;\n>> +\t}\n>> +\n>> +\tif (ret < 0 && !readBytes)\n> \n> the man page for read(2) states:\n> \n> \n> # On error, -1 is returned, and errno is set appropriately. In this\n> # case, it is left unspecified whether the file position (if any)\n> # changes.\n> \n> \n> In the event of an error, that could thus leave pos_ invalid, but I\n> don't think we care too much about that for an internal helper which\n> probably isn't going to use pos much anyway.\n> \n> We could:\n> \n> A) call lseek to explicitly reset pos\n> B) Continue to do nothing.\n\nC) Call lseek directly to obtain the actual pos...\nD) Remove pos_ and follow neg's suggestion ;-)\n\nEven still, once a route forwards on that is addressed this still holds:\n\n> Reviewed-by: Kieran Bingham \n> \n> \n>> +\t\treturn -errno;\n>> +\n>> +\tpos_ += readBytes;\n>> +\treturn readBytes;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Write data to the file\n>> + * \\param[in] data Memory containing data to be written\n>> + *\n>> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n>> + * return the number of bytes written. If the file system doesn't have enough\n>> + * space for the data, the returned byte count may be less than requested.\n>> + *\n>> + * The position of the file as returned by pos() is advanced by the number of\n>> + * bytes written. If an error occurs, the position isn't modified.\n>> + *\n>> + * \\return The number of bytes written on success, or a negative error code\n>> + * otherwise\n>> + */\n>> +ssize_t File::write(const Span &data)\n>> +{\n>> +\tif (!isOpen())\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tsize_t writtenBytes = 0;\n>> +\n>> +\t/* Retry in case of interrupted system calls. */\n>> +\twhile (writtenBytes < data.size()) {\n>> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n>> +\t\t\t\t data.size() - writtenBytes);\n>> +\t\tif (ret <= 0)\n>> +\t\t\tbreak;\n>> +\n>> +\t\twrittenBytes += ret;\n>> +\t}\n>> +\n>> +\tif (data.size() && !writtenBytes)\n>> +\t\treturn -errno;\n>> +\n>> +\tpos_ += writtenBytes;> +\treturn writtenBytes;\n>> +}\n>> +\n>> /**\n>> * \\brief Map a region of the file in the process memory\n>> * \\param[in] offset The region offset within the file\n>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 019EFBD792\n\tfor ;\n\tMon, 13 Jul 2020 08:34:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82F35605B2;\n\tMon, 13 Jul 2020 10:34:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5619A605B1\n\tfor ;\n\tMon, 13 Jul 2020 10:34:32 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9D988C9;\n\tMon, 13 Jul 2020 10:34:31 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"U8HC0hNU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594629271;\n\tbh=gesjWtxwD11KQPNqhMseLfaVV5PN2/iQUuXHYuux9oE=;\n\th=Reply-To:Subject:From:To:References:Date:In-Reply-To:From;\n\tb=U8HC0hNUG1nW6mBbwRMqCBuzMDutvwWlQhFz0/OIzMfV0l6NYyTCc/J0XwT+dotLY\n\tKjAYUdALOZzzne8Xp3Htdt1ShdyNa8h4566HXN784OLSMDmLJyvTg+1m5eEQdSMiUS\n\tP9gFEzLAbP/rEubUzmZlQYWucsgqirU9FqIZ9kDg=","From":"Kieran Bingham ","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>\n\t<7d8b822b-0c5b-e32a-762b-10eac05cc95a@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 13 Jul 2020 09:34:27 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<7d8b822b-0c5b-e32a-762b-10eac05cc95a@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11378,"web_url":"https://patchwork.libcamera.org/comment/11378/","msgid":"<20200713225536.GE25374@pendragon.ideasonboard.com>","date":"2020-07-13T22:55:36","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas, Kieran,\n\nOn Mon, Jul 13, 2020 at 09:31:42AM +0100, Kieran Bingham wrote:\n> Hi Niklas / Laurent,\n> \n> On 13/07/2020 07:53, Niklas Söderlund wrote:\n> > Hi Laurent,\n> > \n> > Thanks for your work.\n> > \n> > On 2020-07-12 17:44:16 +0300, Laurent Pinchart wrote:\n> >> Add basic support to read and write data from/to a file, along with\n> >> retrieving and setting the current read/write position.\n> >>\n> >> Signed-off-by: Laurent Pinchart \n> >> ---\n> >> include/libcamera/internal/file.h | 7 ++\n> >> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n> >> 2 files changed, 116 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n> >> index e3e72132e336..ead94cdf29c9 100644\n> >> --- a/include/libcamera/internal/file.h\n> >> +++ b/include/libcamera/internal/file.h\n> >> @@ -49,6 +49,12 @@ public:\n> >> \tint error() const { return error_; }\n> >> \tssize_t size() const;\n> >> \n> >> +\toff_t pos() const { return pos_; }\n> > \n> > Would it not be nicer to return lseek(fd_, 0, SEEK_CUR) here? That way \n> > we could avoid keeping pos_ in sync in potential corner cases.\n> \n> Aha ... this would of course address my concerns about pos_ becoming\n> invalid on error cases ...\n\nWouldn't it be more efficient to update pos_ in case of a read or write\nerror ? Efficiency may not be our primary concern here though. I don't\nmind much either way, does anyone have a preference ?\n\n> >> +\toff_t seek(off_t pos);\n> >> +\n> >> +\tssize_t read(const Span &data);\n> >> +\tssize_t write(const Span &data);\n> >> +\n> >> \tSpan map(off_t offset = 0, ssize_t size = -1,\n> >> \t\t\t MapFlag flags = MapNoOption);\n> >> \tbool unmap(uint8_t *addr);\n> >> @@ -61,6 +67,7 @@ private:\n> >> \tstd::string name_;\n> >> \tint fd_;\n> >> \tOpenMode mode_;\n> >> +\toff_t pos_;\n> >> \n> >> \tint error_;\n> >> \tstd::map maps_;\n> >> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n> >> index c471bde3fc68..8bd109090919 100644\n> >> --- a/src/libcamera/file.cpp\n> >> +++ b/src/libcamera/file.cpp\n> >> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n> >> * before performing I/O operations.\n> >> */\n> >> File::File(const std::string &name)\n> >> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n> >> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> >> {\n> >> }\n> >> \n> >> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n> >> * setFileName().\n> >> */\n> >> File::File()\n> >> -\t: fd_(-1), mode_(NotOpen), error_(0)\n> >> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> >> {\n> >> }\n> >> \n> >> @@ -202,6 +202,7 @@ void File::close()\n> >> \t::close(fd_);\n> >> \tfd_ = -1;\n> >> \tmode_ = NotOpen;\n> >> +\tpos_ = 0;\n> >> }\n> >> \n> >> /**\n> >> @@ -237,6 +238,112 @@ ssize_t File::size() const\n> >> \treturn st.st_size;\n> >> }\n> >> \n> >> +/**\n> >> + * \\fn off_t File::pos() const\n> >> + * \\brief Return current read or write position\n> >> + *\n> >> + * If the file is closed, this function returns 0.\n> >> + *\n> >> + * \\return The current read or write position\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\brief Set the read or write position\n> >> + * \\param[in] pos The desired position\n> >> + * \\return The resulting offset from the beginning of the file on success, or a\n> >> + * negative error code otherwise\n> >> + */\n> >> +off_t File::seek(off_t pos)\n> >> +{\n> >> +\tif (!isOpen())\n> >> +\t\treturn -EINVAL;\n> >> +\n> >> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n> >> +\tif (ret < 0)\n> >> +\t\treturn -errno;\n> >> +\n> >> +\tpos_ = ret;\n> >> +\treturn ret;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Read data from the file\n> >> + * \\param[in] data Memory to read data into\n> >> + *\n> >> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n> >> + * return the number of bytes read. If less data than requested is available,\n> >> + * the returned byte count may be smaller than the requested size. If no more\n> >> + * data is available, 0 is returned.\n> >> + *\n> >> + * The position of the file as returned by pos() is advanced by the number of\n> >> + * bytes read. If an error occurs, the position isn't modified.\n> >> + *\n> >> + * \\return The number of bytes read on success, or a negative error code\n> >> + * otherwise\n> >> + */\n> >> +ssize_t File::read(const Span &data)\n> >> +{\n> >> +\tif (!isOpen())\n> >> +\t\treturn -EINVAL;\n> >> +\n> >> +\tsize_t readBytes = 0;\n> >> +\tssize_t ret = 0;\n> >> +\n> >> +\t/* Retry in case of interrupted system calls. */\n> >> +\twhile (readBytes < data.size()) {\n> >> +\t\tret = ::read(fd_, data.data() + readBytes,\n> >> +\t\t\t data.size() - readBytes);\n> >> +\t\tif (ret <= 0)\n> >> +\t\t\tbreak;\n> >> +\n> >> +\t\treadBytes += ret;\n> >> +\t}\n> >> +\n> >> +\tif (ret < 0 && !readBytes)\n> >> +\t\treturn -errno;\n> >> +\n> >> +\tpos_ += readBytes;\n> >> +\treturn readBytes;\n> >> +}\n> >> +\n> >> +/**\n> >> + * \\brief Write data to the file\n> >> + * \\param[in] data Memory containing data to be written\n> >> + *\n> >> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n> >> + * return the number of bytes written. If the file system doesn't have enough\n> >> + * space for the data, the returned byte count may be less than requested.\n> >> + *\n> >> + * The position of the file as returned by pos() is advanced by the number of\n> >> + * bytes written. If an error occurs, the position isn't modified.\n> >> + *\n> >> + * \\return The number of bytes written on success, or a negative error code\n> >> + * otherwise\n> >> + */\n> >> +ssize_t File::write(const Span &data)\n> >> +{\n> >> +\tif (!isOpen())\n> >> +\t\treturn -EINVAL;\n> >> +\n> >> +\tsize_t writtenBytes = 0;\n> >> +\n> >> +\t/* Retry in case of interrupted system calls. */\n> >> +\twhile (writtenBytes < data.size()) {\n> >> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n> >> +\t\t\t\t data.size() - writtenBytes);\n> >> +\t\tif (ret <= 0)\n> >> +\t\t\tbreak;\n> >> +\n> >> +\t\twrittenBytes += ret;\n> >> +\t}\n> >> +\n> >> +\tif (data.size() && !writtenBytes)\n> >> +\t\treturn -errno;\n> >> +\n> >> +\tpos_ += writtenBytes;\n> >> +\treturn writtenBytes;\n> >> +}\n> >> +\n> >> /**\n> >> * \\brief Map a region of the file in the process memory\n> >> * \\param[in] offset The region offset within the file","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F0308BD792\n\tfor ;\n\tMon, 13 Jul 2020 22:55:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8BEEB60738;\n\tTue, 14 Jul 2020 00:55:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F20B56048D\n\tfor ;\n\tTue, 14 Jul 2020 00:55:42 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 78E7BC9;\n\tTue, 14 Jul 2020 00:55:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Zzb2tHGr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594680942;\n\tbh=p19JnxGRri07a1EY/PFV/fTVmkVFAyM2o+BcwYYSmfo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Zzb2tHGr9vI0cRUKb/Ur+lKgdBQMiEIe7sRysxZAdKpbuMPmu1RVLkdowbrNfJ1xN\n\tocGwmSsayFR+aZHH7o5HC+DThBr/o99aRUITTronIsjuv+zWD8VX9QD+IYw1/kTR4a\n\t/S8x6j1Rdt55uNoMkS8qv8RZ0R7J9NRpHExam+UQ=","Date":"Tue, 14 Jul 2020 01:55:36 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"<20200713225536.GE25374@pendragon.ideasonboard.com>","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>\n\t<20200713065349.GE1498036@oden.dyn.berto.se>\n\t<7532dd8a-a002-7813-8fbc-ee7dc34a0e8f@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<7532dd8a-a002-7813-8fbc-ee7dc34a0e8f@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11379,"web_url":"https://patchwork.libcamera.org/comment/11379/","msgid":"<20200714055258.GA3051471@oden.dyn.berto.se>","date":"2020-07-14T05:52:58","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nOn 2020-07-14 01:55:36 +0300, Laurent Pinchart wrote:\n> Hi Niklas, Kieran,\n> \n> On Mon, Jul 13, 2020 at 09:31:42AM +0100, Kieran Bingham wrote:\n> > Hi Niklas / Laurent,\n> > \n> > On 13/07/2020 07:53, Niklas Söderlund wrote:\n> > > Hi Laurent,\n> > > \n> > > Thanks for your work.\n> > > \n> > > On 2020-07-12 17:44:16 +0300, Laurent Pinchart wrote:\n> > >> Add basic support to read and write data from/to a file, along with\n> > >> retrieving and setting the current read/write position.\n> > >>\n> > >> Signed-off-by: Laurent Pinchart \n> > >> ---\n> > >> include/libcamera/internal/file.h | 7 ++\n> > >> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n> > >> 2 files changed, 116 insertions(+), 2 deletions(-)\n> > >>\n> > >> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n> > >> index e3e72132e336..ead94cdf29c9 100644\n> > >> --- a/include/libcamera/internal/file.h\n> > >> +++ b/include/libcamera/internal/file.h\n> > >> @@ -49,6 +49,12 @@ public:\n> > >> \tint error() const { return error_; }\n> > >> \tssize_t size() const;\n> > >> \n> > >> +\toff_t pos() const { return pos_; }\n> > > \n> > > Would it not be nicer to return lseek(fd_, 0, SEEK_CUR) here? That way \n> > > we could avoid keeping pos_ in sync in potential corner cases.\n> > \n> > Aha ... this would of course address my concerns about pos_ becoming\n> > invalid on error cases ...\n> \n> Wouldn't it be more efficient to update pos_ in case of a read or write\n> error ? Efficiency may not be our primary concern here though. I don't\n> mind much either way, does anyone have a preference ?\n\nNo strong preference, but if the kernel keeps track of things for you it \nseems folly to reimplement it in user-space :-)\n\nWhatever counters the kernel keep to make SEEK_CUR work it will still do \ndisregarding if we use it or not. So the only efficiency gain would be \nif we read it often and modified it a seldom, in that case I rather just \ncache SEEK_CUR and refresh it each time we to a write() or read().\n\n> \n> > >> +\toff_t seek(off_t pos);\n> > >> +\n> > >> +\tssize_t read(const Span &data);\n> > >> +\tssize_t write(const Span &data);\n> > >> +\n> > >> \tSpan map(off_t offset = 0, ssize_t size = -1,\n> > >> \t\t\t MapFlag flags = MapNoOption);\n> > >> \tbool unmap(uint8_t *addr);\n> > >> @@ -61,6 +67,7 @@ private:\n> > >> \tstd::string name_;\n> > >> \tint fd_;\n> > >> \tOpenMode mode_;\n> > >> +\toff_t pos_;\n> > >> \n> > >> \tint error_;\n> > >> \tstd::map maps_;\n> > >> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n> > >> index c471bde3fc68..8bd109090919 100644\n> > >> --- a/src/libcamera/file.cpp\n> > >> +++ b/src/libcamera/file.cpp\n> > >> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n> > >> * before performing I/O operations.\n> > >> */\n> > >> File::File(const std::string &name)\n> > >> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n> > >> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> > >> {\n> > >> }\n> > >> \n> > >> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n> > >> * setFileName().\n> > >> */\n> > >> File::File()\n> > >> -\t: fd_(-1), mode_(NotOpen), error_(0)\n> > >> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n> > >> {\n> > >> }\n> > >> \n> > >> @@ -202,6 +202,7 @@ void File::close()\n> > >> \t::close(fd_);\n> > >> \tfd_ = -1;\n> > >> \tmode_ = NotOpen;\n> > >> +\tpos_ = 0;\n> > >> }\n> > >> \n> > >> /**\n> > >> @@ -237,6 +238,112 @@ ssize_t File::size() const\n> > >> \treturn st.st_size;\n> > >> }\n> > >> \n> > >> +/**\n> > >> + * \\fn off_t File::pos() const\n> > >> + * \\brief Return current read or write position\n> > >> + *\n> > >> + * If the file is closed, this function returns 0.\n> > >> + *\n> > >> + * \\return The current read or write position\n> > >> + */\n> > >> +\n> > >> +/**\n> > >> + * \\brief Set the read or write position\n> > >> + * \\param[in] pos The desired position\n> > >> + * \\return The resulting offset from the beginning of the file on success, or a\n> > >> + * negative error code otherwise\n> > >> + */\n> > >> +off_t File::seek(off_t pos)\n> > >> +{\n> > >> +\tif (!isOpen())\n> > >> +\t\treturn -EINVAL;\n> > >> +\n> > >> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n> > >> +\tif (ret < 0)\n> > >> +\t\treturn -errno;\n> > >> +\n> > >> +\tpos_ = ret;\n> > >> +\treturn ret;\n> > >> +}\n> > >> +\n> > >> +/**\n> > >> + * \\brief Read data from the file\n> > >> + * \\param[in] data Memory to read data into\n> > >> + *\n> > >> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n> > >> + * return the number of bytes read. If less data than requested is available,\n> > >> + * the returned byte count may be smaller than the requested size. If no more\n> > >> + * data is available, 0 is returned.\n> > >> + *\n> > >> + * The position of the file as returned by pos() is advanced by the number of\n> > >> + * bytes read. If an error occurs, the position isn't modified.\n> > >> + *\n> > >> + * \\return The number of bytes read on success, or a negative error code\n> > >> + * otherwise\n> > >> + */\n> > >> +ssize_t File::read(const Span &data)\n> > >> +{\n> > >> +\tif (!isOpen())\n> > >> +\t\treturn -EINVAL;\n> > >> +\n> > >> +\tsize_t readBytes = 0;\n> > >> +\tssize_t ret = 0;\n> > >> +\n> > >> +\t/* Retry in case of interrupted system calls. */\n> > >> +\twhile (readBytes < data.size()) {\n> > >> +\t\tret = ::read(fd_, data.data() + readBytes,\n> > >> +\t\t\t data.size() - readBytes);\n> > >> +\t\tif (ret <= 0)\n> > >> +\t\t\tbreak;\n> > >> +\n> > >> +\t\treadBytes += ret;\n> > >> +\t}\n> > >> +\n> > >> +\tif (ret < 0 && !readBytes)\n> > >> +\t\treturn -errno;\n> > >> +\n> > >> +\tpos_ += readBytes;\n> > >> +\treturn readBytes;\n> > >> +}\n> > >> +\n> > >> +/**\n> > >> + * \\brief Write data to the file\n> > >> + * \\param[in] data Memory containing data to be written\n> > >> + *\n> > >> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n> > >> + * return the number of bytes written. If the file system doesn't have enough\n> > >> + * space for the data, the returned byte count may be less than requested.\n> > >> + *\n> > >> + * The position of the file as returned by pos() is advanced by the number of\n> > >> + * bytes written. If an error occurs, the position isn't modified.\n> > >> + *\n> > >> + * \\return The number of bytes written on success, or a negative error code\n> > >> + * otherwise\n> > >> + */\n> > >> +ssize_t File::write(const Span &data)\n> > >> +{\n> > >> +\tif (!isOpen())\n> > >> +\t\treturn -EINVAL;\n> > >> +\n> > >> +\tsize_t writtenBytes = 0;\n> > >> +\n> > >> +\t/* Retry in case of interrupted system calls. */\n> > >> +\twhile (writtenBytes < data.size()) {\n> > >> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n> > >> +\t\t\t\t data.size() - writtenBytes);\n> > >> +\t\tif (ret <= 0)\n> > >> +\t\t\tbreak;\n> > >> +\n> > >> +\t\twrittenBytes += ret;\n> > >> +\t}\n> > >> +\n> > >> +\tif (data.size() && !writtenBytes)\n> > >> +\t\treturn -errno;\n> > >> +\n> > >> +\tpos_ += writtenBytes;\n> > >> +\treturn writtenBytes;\n> > >> +}\n> > >> +\n> > >> /**\n> > >> * \\brief Map a region of the file in the process memory\n> > >> * \\param[in] offset The region offset within the file\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D3BF8BD790\n\tfor ;\n\tTue, 14 Jul 2020 05:53:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5412060553;\n\tTue, 14 Jul 2020 07:53:03 +0200 (CEST)","from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C5CED6048B\n\tfor ;\n\tTue, 14 Jul 2020 07:53:01 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id e8so21019087ljb.0\n\tfor ;\n\tMon, 13 Jul 2020 22:53:01 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tf14sm5073061lfa.35.2020.07.13.22.52.59\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 13 Jul 2020 22:52:59 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"t7vCoZxE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=BIkjguDJsI+79qzKAdyH3qJ6Rkeu7ckc3EXnNkQx2w4=;\n\tb=t7vCoZxEZG4fJapDjXr0lVN16p79mxDRokQzOv/0cDgXGgYoCANfFk5r2K/AwAC+Cc\n\tV7VtL/0s7pVFDDe3Ngcht+KLspru/7UF6BSXgAvLaT/hQOwkvhynN7ufqwEdv63Ui4BU\n\tv3RsEJsx0C4/8Q6QMmH9LDTOToUqmcHimwd9BuKBARSPN1jgaXyKtHEBNH+WbH3EXPei\n\tjOpnDRoEWZNAY5/AuRuJvuAWBBZ7dCMAt4YkhWbRFxj9neAcWRIbVoVllbgJ/2bkpz1S\n\t6qO5TN+l5GG8Czz2pA1vNKQBx3Gh08ZyWnHobYaDB00aM31cIX1xUXLw4vz1FTGElQ51\n\toDEQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=BIkjguDJsI+79qzKAdyH3qJ6Rkeu7ckc3EXnNkQx2w4=;\n\tb=n4M55rfvDfbpMfqVTPvS7b3J3UIC9taOm0vMnFBARnttceu0X2nzMs56HKr6ojmF7T\n\tgMmRD1UwXgGzN05l/ibqraRWJzR23Dk2ce/9ZdlwYWgWv4fpowCWoXR3CxHDX6wuzV2R\n\tKIlr2ARTAulOxIW8BpKou/RUFzgfjmZEivRWoJRaR+qb/zeYwNcGdJCXMivLj8KJY6Hw\n\tRKyx5Wb+FWYGul453SEpidL18nscdyHyJJC+kYJ8eYgjufRfKaKFPBo4qyfzuLBihxom\n\tVPVgP0ZPWcYt85WPPFuIE9TLAzcTPlEU8dEYCkQb/QRcKeiZVTDrJ15tkkUDzoP1e7W8\n\tGGjg==","X-Gm-Message-State":"AOAM531fue+Z6OplbtviuK8ixoQiC8OKFRrRkYVKo/eu3XuK/r0dFpAR\n\tZk9WicOrrwljy7xQj4u/J/kROYXzRao=","X-Google-Smtp-Source":"ABdhPJzzHu7mEWQV7LdRWLN73jhWbl/u7qyU3rE3hLi9KI9cuUQ6jijgOvJ/rooAQGH5yqFw8rBcRw==","X-Received":"by 2002:a2e:b8ce:: with SMTP id s14mr1376915ljp.89.1594705980567;\n\tMon, 13 Jul 2020 22:53:00 -0700 (PDT)","Date":"Tue, 14 Jul 2020 07:52:58 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"<20200714055258.GA3051471@oden.dyn.berto.se>","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>\n\t<20200713065349.GE1498036@oden.dyn.berto.se>\n\t<7532dd8a-a002-7813-8fbc-ee7dc34a0e8f@ideasonboard.com>\n\t<20200713225536.GE25374@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200713225536.GE25374@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":11380,"web_url":"https://patchwork.libcamera.org/comment/11380/","msgid":"<6e12eb95-ccbd-8bd3-6030-ae321f7dc001@ideasonboard.com>","date":"2020-07-14T07:59:03","subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi,\n\nOn 14/07/2020 06:52, Niklas Söderlund wrote:\n> Hi Laurent,\n> \n> On 2020-07-14 01:55:36 +0300, Laurent Pinchart wrote:\n>> Hi Niklas, Kieran,\n>>\n>> On Mon, Jul 13, 2020 at 09:31:42AM +0100, Kieran Bingham wrote:\n>>> Hi Niklas / Laurent,\n>>>\n>>> On 13/07/2020 07:53, Niklas Söderlund wrote:\n>>>> Hi Laurent,\n>>>>\n>>>> Thanks for your work.\n>>>>\n>>>> On 2020-07-12 17:44:16 +0300, Laurent Pinchart wrote:\n>>>>> Add basic support to read and write data from/to a file, along with\n>>>>> retrieving and setting the current read/write position.\n>>>>>\n>>>>> Signed-off-by: Laurent Pinchart \n>>>>> ---\n>>>>> include/libcamera/internal/file.h | 7 ++\n>>>>> src/libcamera/file.cpp | 111 +++++++++++++++++++++++++++++-\n>>>>> 2 files changed, 116 insertions(+), 2 deletions(-)\n>>>>>\n>>>>> diff --git a/include/libcamera/internal/file.h b/include/libcamera/internal/file.h\n>>>>> index e3e72132e336..ead94cdf29c9 100644\n>>>>> --- a/include/libcamera/internal/file.h\n>>>>> +++ b/include/libcamera/internal/file.h\n>>>>> @@ -49,6 +49,12 @@ public:\n>>>>> \tint error() const { return error_; }\n>>>>> \tssize_t size() const;\n>>>>> \n>>>>> +\toff_t pos() const { return pos_; }\n>>>>\n>>>> Would it not be nicer to return lseek(fd_, 0, SEEK_CUR) here? That way \n>>>> we could avoid keeping pos_ in sync in potential corner cases.\n>>>\n>>> Aha ... this would of course address my concerns about pos_ becoming\n>>> invalid on error cases ...\n>>\n>> Wouldn't it be more efficient to update pos_ in case of a read or write\n>> error ? Efficiency may not be our primary concern here though. I don't\n>> mind much either way, does anyone have a preference ?\n> \n> No strong preference, but if the kernel keeps track of things for you it \n> seems folly to reimplement it in user-space :-)\n> \n> Whatever counters the kernel keep to make SEEK_CUR work it will still do \n> disregarding if we use it or not. So the only efficiency gain would be \n> if we read it often and modified it a seldom, in that case I rather just \n> cache SEEK_CUR and refresh it each time we to a write() or read().\n\nMy feel is that it's keeping a cached value of the position in\nuserspace, which is rarely (if ever) used, and not expected to be a\nperformance gain to cache it, /and/ it can be wrong in error cases, and\nthus need updating.\n\nIMO, it's likely adding code for the sake of it.\nI'd go for just a direct call to lseek to obtain pos() when requested,\nwhich is probably just a helper and doesn't have any callers anyway?\n\nIf that ever became a performance bottle neck, it could be cached, but I\nreally can't see that being a case we're going to experience in this\ninstance.\n\nDo we even need to provide pos() at all?\n\nI don't mind adding it as a generic API function on the libcamera::File\nAPI, but it's internal anyway?\n\nPerhaps we'll see more use of the libcamera::File api for\nreading/writing configuration files ... is that where you anticipate\nneeding to obtain the current pos()?\n\n--\nKieran\n\n\n\n\n>>>>> +\toff_t seek(off_t pos);\n>>>>> +\n>>>>> +\tssize_t read(const Span &data);\n>>>>> +\tssize_t write(const Span &data);\n>>>>> +\n>>>>> \tSpan map(off_t offset = 0, ssize_t size = -1,\n>>>>> \t\t\t MapFlag flags = MapNoOption);\n>>>>> \tbool unmap(uint8_t *addr);\n>>>>> @@ -61,6 +67,7 @@ private:\n>>>>> \tstd::string name_;\n>>>>> \tint fd_;\n>>>>> \tOpenMode mode_;\n>>>>> +\toff_t pos_;\n>>>>> \n>>>>> \tint error_;\n>>>>> \tstd::map maps_;\n>>>>> diff --git a/src/libcamera/file.cpp b/src/libcamera/file.cpp\n>>>>> index c471bde3fc68..8bd109090919 100644\n>>>>> --- a/src/libcamera/file.cpp\n>>>>> +++ b/src/libcamera/file.cpp\n>>>>> @@ -73,7 +73,7 @@ LOG_DEFINE_CATEGORY(File);\n>>>>> * before performing I/O operations.\n>>>>> */\n>>>>> File::File(const std::string &name)\n>>>>> -\t: name_(name), fd_(-1), mode_(NotOpen), error_(0)\n>>>>> +\t: name_(name), fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n>>>>> {\n>>>>> }\n>>>>> \n>>>>> @@ -84,7 +84,7 @@ File::File(const std::string &name)\n>>>>> * setFileName().\n>>>>> */\n>>>>> File::File()\n>>>>> -\t: fd_(-1), mode_(NotOpen), error_(0)\n>>>>> +\t: fd_(-1), mode_(NotOpen), pos_(0), error_(0)\n>>>>> {\n>>>>> }\n>>>>> \n>>>>> @@ -202,6 +202,7 @@ void File::close()\n>>>>> \t::close(fd_);\n>>>>> \tfd_ = -1;\n>>>>> \tmode_ = NotOpen;\n>>>>> +\tpos_ = 0;\n>>>>> }\n>>>>> \n>>>>> /**\n>>>>> @@ -237,6 +238,112 @@ ssize_t File::size() const\n>>>>> \treturn st.st_size;\n>>>>> }\n>>>>> \n>>>>> +/**\n>>>>> + * \\fn off_t File::pos() const\n>>>>> + * \\brief Return current read or write position\n>>>>> + *\n>>>>> + * If the file is closed, this function returns 0.\n>>>>> + *\n>>>>> + * \\return The current read or write position\n>>>>> + */\n>>>>> +\n>>>>> +/**\n>>>>> + * \\brief Set the read or write position\n>>>>> + * \\param[in] pos The desired position\n>>>>> + * \\return The resulting offset from the beginning of the file on success, or a\n>>>>> + * negative error code otherwise\n>>>>> + */\n>>>>> +off_t File::seek(off_t pos)\n>>>>> +{\n>>>>> +\tif (!isOpen())\n>>>>> +\t\treturn -EINVAL;\n>>>>> +\n>>>>> +\toff_t ret = lseek(fd_, pos, SEEK_SET);\n>>>>> +\tif (ret < 0)\n>>>>> +\t\treturn -errno;\n>>>>> +\n>>>>> +\tpos_ = ret;\n>>>>> +\treturn ret;\n>>>>> +}\n>>>>> +\n>>>>> +/**\n>>>>> + * \\brief Read data from the file\n>>>>> + * \\param[in] data Memory to read data into\n>>>>> + *\n>>>>> + * Read at most \\a data.size() bytes from the file into \\a data.data(), and\n>>>>> + * return the number of bytes read. If less data than requested is available,\n>>>>> + * the returned byte count may be smaller than the requested size. If no more\n>>>>> + * data is available, 0 is returned.\n>>>>> + *\n>>>>> + * The position of the file as returned by pos() is advanced by the number of\n>>>>> + * bytes read. If an error occurs, the position isn't modified.\n>>>>> + *\n>>>>> + * \\return The number of bytes read on success, or a negative error code\n>>>>> + * otherwise\n>>>>> + */\n>>>>> +ssize_t File::read(const Span &data)\n>>>>> +{\n>>>>> +\tif (!isOpen())\n>>>>> +\t\treturn -EINVAL;\n>>>>> +\n>>>>> +\tsize_t readBytes = 0;\n>>>>> +\tssize_t ret = 0;\n>>>>> +\n>>>>> +\t/* Retry in case of interrupted system calls. */\n>>>>> +\twhile (readBytes < data.size()) {\n>>>>> +\t\tret = ::read(fd_, data.data() + readBytes,\n>>>>> +\t\t\t data.size() - readBytes);\n>>>>> +\t\tif (ret <= 0)\n>>>>> +\t\t\tbreak;\n>>>>> +\n>>>>> +\t\treadBytes += ret;\n>>>>> +\t}\n>>>>> +\n>>>>> +\tif (ret < 0 && !readBytes)\n>>>>> +\t\treturn -errno;\n>>>>> +\n>>>>> +\tpos_ += readBytes;\n>>>>> +\treturn readBytes;\n>>>>> +}\n>>>>> +\n>>>>> +/**\n>>>>> + * \\brief Write data to the file\n>>>>> + * \\param[in] data Memory containing data to be written\n>>>>> + *\n>>>>> + * Write at most \\a data.size() bytes from \\a data.data() to the file, and\n>>>>> + * return the number of bytes written. If the file system doesn't have enough\n>>>>> + * space for the data, the returned byte count may be less than requested.\n>>>>> + *\n>>>>> + * The position of the file as returned by pos() is advanced by the number of\n>>>>> + * bytes written. If an error occurs, the position isn't modified.\n>>>>> + *\n>>>>> + * \\return The number of bytes written on success, or a negative error code\n>>>>> + * otherwise\n>>>>> + */\n>>>>> +ssize_t File::write(const Span &data)\n>>>>> +{\n>>>>> +\tif (!isOpen())\n>>>>> +\t\treturn -EINVAL;\n>>>>> +\n>>>>> +\tsize_t writtenBytes = 0;\n>>>>> +\n>>>>> +\t/* Retry in case of interrupted system calls. */\n>>>>> +\twhile (writtenBytes < data.size()) {\n>>>>> +\t\tssize_t ret = ::write(fd_, data.data() + writtenBytes,\n>>>>> +\t\t\t\t data.size() - writtenBytes);\n>>>>> +\t\tif (ret <= 0)\n>>>>> +\t\t\tbreak;\n>>>>> +\n>>>>> +\t\twrittenBytes += ret;\n>>>>> +\t}\n>>>>> +\n>>>>> +\tif (data.size() && !writtenBytes)\n>>>>> +\t\treturn -errno;\n>>>>> +\n>>>>> +\tpos_ += writtenBytes;\n>>>>> +\treturn writtenBytes;\n>>>>> +}\n>>>>> +\n>>>>> /**\n>>>>> * \\brief Map a region of the file in the process memory\n>>>>> * \\param[in] offset The region offset within the file\n>>\n>> -- \n>> Regards,\n>>\n>> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6307DBD792\n\tfor ;\n\tTue, 14 Jul 2020 07:59:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8EC760738;\n\tTue, 14 Jul 2020 09:59:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2EA4E605AC\n\tfor ;\n\tTue, 14 Jul 2020 09:59:08 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 81DD3564;\n\tTue, 14 Jul 2020 09:59:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bU0K/HOd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1594713547;\n\tbh=4hRrWeaPiyRQnMcT54N7L/RWdHedEHRqmpC5aDrA9mQ=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=bU0K/HOdO5w31rfrt+0h/iYkrzKEZQacUbXs90m5MCB52pxO+V1FGy0V83PKGT4P6\n\t48mfWPrg6LDJEo1CItCo7dezOYmAyI9X172FsfyKoU2hd5Ka8LGwtyCYr/rfViRy4Q\n\txCsq2mqOGJtiBEMpK8bcXdjC73DQuie9sdeorM80=","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tLaurent Pinchart ","References":"<20200712144419.21457-1-laurent.pinchart@ideasonboard.com>\n\t<20200713065349.GE1498036@oden.dyn.berto.se>\n\t<7532dd8a-a002-7813-8fbc-ee7dc34a0e8f@ideasonboard.com>\n\t<20200713225536.GE25374@pendragon.ideasonboard.com>\n\t<20200714055258.GA3051471@oden.dyn.berto.se>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<6e12eb95-ccbd-8bd3-6030-ae321f7dc001@ideasonboard.com>","Date":"Tue, 14 Jul 2020 08:59:03 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.8.0","MIME-Version":"1.0","In-Reply-To":"<20200714055258.GA3051471@oden.dyn.berto.se>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 1/4] libcamera: file: Add read/write\n\tsupport","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]