changeset 78:d58e45e3515f

Irccd: various improvements in Irccd.File API, #446, #447 - Irccd.File.read should be much faster in various ways, - Less usages of exceptions, - Do not export File object API, - Style.
author David Demelier <markand@malikania.fr>
date Wed, 30 Mar 2016 20:56:22 +0200
parents c5f4ca941f79
children d0deb5835aed
files doc/html/api/module/Irccd.File/method/write.md lib/irccd/js-file.cpp lib/irccd/js-file.h
diffstat 3 files changed, 114 insertions(+), 217 deletions(-) [+]
line wrap: on
line diff
--- a/doc/html/api/module/Irccd.File/method/write.md	Wed Mar 30 19:01:53 2016 +0200
+++ b/doc/html/api/module/Irccd.File/method/write.md	Wed Mar 30 20:56:22 2016 +0200
@@ -4,5 +4,6 @@
 synopsis: "File.prototype.write(data)"
 arguments:
   - "**data**: the character to write."
+returns: "The number of bytes written."
 throws: "An [Irccd.SystemError](@baseurl@/api/module/Irccd/index.html#types) on failures."
 ---
--- a/lib/irccd/js-file.cpp	Wed Mar 30 19:01:53 2016 +0200
+++ b/lib/irccd/js-file.cpp	Wed Mar 30 20:56:22 2016 +0200
@@ -16,9 +16,8 @@
  * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-#include <cerrno>
-#include <cstring>
-#include <fstream>
+#include <array>
+#include <vector>
 
 #include <irccd-config.h>
 
@@ -36,105 +35,6 @@
 namespace irccd {
 
 /*
- * duk::File object for Javascript I/O
- * ------------------------------------------------------------------
- */
-
-File::File(std::string path, const std::string &mode)
-	: m_path(std::move(path))
-	, m_destructor([] (std::FILE *fp) { std::fclose(fp); })
-{
-	if ((m_stream = std::fopen(m_path.c_str(), mode.c_str())) == nullptr)
-		throw std::runtime_error(std::strerror(errno));
-}
-
-File::File(std::FILE *fp, std::function<void (std::FILE *)> destructor) noexcept
-	: m_stream(fp)
-	, m_destructor(std::move(destructor))
-{
-	assert(m_destructor != nullptr);
-}
-
-File::~File() noexcept
-{
-	close();
-}
-
-void File::close() noexcept
-{
-	if (m_stream) {
-		m_destructor(m_stream);
-		m_stream = nullptr;
-	}
-}
-
-bool File::isClosed() noexcept
-{
-	return m_stream == nullptr;
-}
-
-void File::seek(long amount, long dir)
-{
-	if (std::fseek(m_stream, amount, dir) != 0)
-		throw std::runtime_error(std::strerror(errno));
-}
-
-unsigned File::tell()
-{
-	long pos = std::ftell(m_stream);
-
-	if (pos == -1L)
-		throw std::runtime_error(std::strerror(errno));
-
-	return pos;
-}
-
-std::string File::readline()
-{
-	std::string result;
-	int ch;
-
-	while ((ch = std::fgetc(m_stream)) != EOF && ch != '\n')
-		result += ch;
-
-	if (ch == EOF && std::ferror(m_stream))
-		throw std::runtime_error(std::strerror(errno));
-
-	return result;
-}
-
-std::string File::read(int amount)
-{
-	assert(amount != 0);
-
-	std::string result;
-	int ch;
-
-	for (int i = 0; (ch = std::fgetc(m_stream)) != EOF; ) {
-		result += ch;
-
-		if (amount > 0 && ++i == amount)
-			break;
-	}
-
-	if (ch == EOF && std::ferror(m_stream))
-		throw std::runtime_error(std::strerror(errno));
-
-	return result;
-}
-
-void File::write(const std::string &data)
-{
-	if (std::fwrite(data.c_str(), data.length(), 1, m_stream) != 1)
-		throw std::runtime_error(std::strerror(errno));
-}
-
-bool File::eof() const noexcept
-{
-	return std::feof(m_stream);
-}
-
-/*
  * duk::TypeInfo specialization for struct stat
  * ------------------------------------------------------------------
  */
@@ -196,9 +96,10 @@
 
 namespace {
 
-/* --------------------------------------------------------
- * File methods
- * -------------------------------------------------------- */
+/*
+ * Anonymous helpers.
+ * ------------------------------------------------------------------
+ */
 
 /* Remove trailing \r for CRLF line style */
 inline std::string clearCr(std::string input)
@@ -210,6 +111,11 @@
 }
 
 /*
+ * File methods.
+ * ------------------------------------------------------------------
+ */
+
+/*
  * Method: File.basename()
  * --------------------------------------------------------
  *
@@ -271,17 +177,16 @@
 
 	std::FILE *fp = duk::self<duk::Pointer<File>>(ctx)->handle();
 	std::string buffer;
+	std::array<char, 128> data;
+	std::int32_t i = 0;
 
-	char data[128];
-	int total = 0;
-
-	while (std::fgets(data, sizeof (data), fp) != nullptr) {
-		buffer += data;
+	while (std::fgets(&data[0], data.size(), fp) != nullptr) {
+		buffer += data.data();
 
 		auto pos = buffer.find('\n');
 
 		if (pos != std::string::npos) {
-			duk::putProperty(ctx, -1, total++, clearCr(buffer.substr(0, pos)));
+			duk::putProperty(ctx, -1, i++, clearCr(buffer.substr(0, pos)));
 			buffer.erase(0, pos + 1);
 		}
 	}
@@ -292,7 +197,7 @@
 
 	/* Missing '\n' in end of file */
 	if (!buffer.empty())
-		duk::putProperty(ctx, -1, total++, clearCr(buffer));
+		duk::putProperty(ctx, -1, i++, clearCr(buffer));
 
 	return 1;
 }
@@ -313,13 +218,33 @@
 duk::Ret methodRead(duk::ContextPtr ctx)
 {
 	auto amount = duk::optional<int>(ctx, 0, -1);
-	auto self = duk::self<duk::Pointer<File>>(ctx);
+	auto file = duk::self<duk::Pointer<File>>(ctx);
 
-	if (amount == 0 || self->isClosed())
+	if (amount == 0 || file->handle() == nullptr)
 		return 0;
 
 	try {
-		duk::push(ctx, self->read(amount));
+		std::string data;
+		std::size_t total = 0;
+
+		if (amount < 0) {
+			std::array<char, 128> buffer;
+			std::size_t nread;
+
+			while ((nread = std::fread(&buffer[0], sizeof (buffer[0]), buffer.size(), file->handle())) > 0) {
+				data += std::string(buffer.data(), nread);
+				total += nread;
+			}
+		} else {
+			data.resize((std::size_t)amount);
+			total = std::fread(&data[0], sizeof (data[0]), (std::size_t)amount, file->handle());
+			data.resize(total);
+		}
+
+		if (std::ferror(file->handle()))
+			duk::raise(ctx, SystemError());
+
+		duk::push(ctx, std::string(data.data(), total));
 	} catch (const std::exception &) {
 		duk::raise(ctx, SystemError());
 	}
@@ -340,16 +265,19 @@
  */
 duk::Ret methodReadline(duk::ContextPtr ctx)
 {
-	try {
-		auto file = duk::self<duk::Pointer<File>>(ctx);
+	std::FILE *fp = duk::self<duk::Pointer<File>>(ctx)->handle();
+	std::string result;
+
+	if (fp == nullptr || std::feof(fp))
+		return 0;
 
-		if (file->isClosed() || file->eof())
-			return 0;
+	for (int ch; (ch = std::fgetc(fp)) != EOF && ch != '\n'; )
+		result += (char)ch;
 
-		duk::push(ctx, clearCr(file->readline()));
-	} catch (const std::exception &) {
+	if (std::ferror(fp))
 		duk::raise(ctx, SystemError());
-	}
+
+	duk::push(ctx, clearCr(result));
 
 	return 1;
 }
@@ -387,16 +315,10 @@
 {
 	auto type = duk::require<int>(ctx, 0);
 	auto amount = duk::require<int>(ctx, 1);
-	auto file = duk::self<duk::Pointer<File>>(ctx);
-
-	if (file->isClosed())
-		return 0;
+	auto fp = duk::self<duk::Pointer<File>>(ctx)->handle();
 
-	try {
-		file->seek(amount, type);
-	} catch (const std::exception &) {
+	if (fp != nullptr && std::fseek(fp, amount, type) != 0)
 		duk::raise(ctx, SystemError());
-	}
 
 	return 0;
 }
@@ -419,13 +341,10 @@
 	struct stat st;
 	auto file = duk::self<duk::Pointer<File>>(ctx);
 
-	if (file->isClosed())
-		return 0;
-
-	if (::stat(file->path().c_str(), &st) < 0)
+	if (file->handle() == nullptr && ::stat(file->path().c_str(), &st) < 0)
 		duk::raise(ctx, SystemError());
-
-	duk::push(ctx, st);
+	else
+		duk::push(ctx, st);
 
 	return 1;
 }
@@ -445,16 +364,16 @@
  */
 duk::Ret methodTell(duk::ContextPtr ctx)
 {
-	auto file = duk::self<duk::Pointer<File>>(ctx);
+	auto fp = duk::self<duk::Pointer<File>>(ctx)->handle();
+	long pos;
 
-	if (file->isClosed())
+	if (fp == nullptr)
 		return 0;
 
-	try {
-		duk::push(ctx, static_cast<int>(file->tell()));
-	} catch (const std::exception &) {
+	if ((pos = std::ftell(fp)) == -1L)
 		duk::raise(ctx, SystemError());
-	}
+	else
+		duk::push(ctx, (int)pos);
 
 	return 1;
 }
@@ -467,23 +386,27 @@
  *
  * Arguments:
  *   - data, the character to write.
+ * Returns:
+ *   The number of bytes written.
  * Throws:
  *   - Any exception on error.
  */
 duk::Ret methodWrite(duk::ContextPtr ctx)
 {
-	auto file = duk::self<duk::Pointer<File>>(ctx);
+	std::FILE *fp = duk::self<duk::Pointer<File>>(ctx)->handle();
+	std::string data = duk::require<std::string>(ctx, 0);
 
-	if (file->isClosed())
+	if (fp == nullptr)
 		return 0;
 
-	try {
-		file->write(duk::require<std::string>(ctx, 0));
-	} catch (const std::exception &) {
+	std::size_t nwritten = std::fwrite(data.c_str(), 1, data.length(), fp);
+
+	if (std::ferror(fp))
 		duk::raise(ctx, SystemError());
-	}
 
-	return 0;
+	duk::push(ctx, (int)nwritten);
+
+	return 1;
 }
 
 const duk::FunctionMap methods{
@@ -502,9 +425,10 @@
 	{ "write",	{ methodWrite,		1	} },
 };
 
-/* --------------------------------------------------------
+/*
  * File "static" functions
- * -------------------------------------------------------- */
+ * ------------------------------------------------------------------
+ */
 
 /*
  * Function: Irccd.File(path, mode) [constructor]
@@ -650,9 +574,9 @@
 };
 
 const duk::Map<int> constants{
-	{ "SeekCur",	static_cast<int>(std::fstream::cur)	},
-	{ "SeekEnd",	static_cast<int>(std::fstream::end)	},
-	{ "SeekSet",	static_cast<int>(std::fstream::beg)	},
+	{ "SeekCur",	SEEK_CUR },
+	{ "SeekEnd",	SEEK_END },
+	{ "SeekSet",	SEEK_SET },
 };
 
 } // !namespace
--- a/lib/irccd/js-file.h	Wed Mar 30 19:01:53 2016 +0200
+++ b/lib/irccd/js-file.h	Wed Mar 30 20:56:22 2016 +0200
@@ -19,7 +19,13 @@
 #ifndef IRCCD_JS_FILE_H
 #define IRCCD_JS_FILE_H
 
+#include <cassert>
+#include <cerrno>
 #include <cstdio>
+#include <cstring>
+#include <functional>
+#include <stdexcept>
+#include <string>
 
 #include "js.h"
 
@@ -57,7 +63,13 @@
 	 * @param mode the mode string (for std::fopen)
 	 * @throw std::runtime_error on failures
 	 */
-	File(std::string path, const std::string &mode);
+	inline File(std::string path, const std::string &mode)
+		: m_path(std::move(path))
+		, m_destructor([] (std::FILE *fp) { std::fclose(fp); })
+	{
+		if ((m_stream = std::fopen(m_path.c_str(), mode.c_str())) == nullptr)
+			throw std::runtime_error(std::strerror(errno));
+	}
 
 	/**
 	 * Construct a file from a already created FILE pointer (e.g. popen).
@@ -67,12 +79,20 @@
 	 * @pre destructor must not be null
 	 * @param fp the file pointer
 	 */
-	File(std::FILE *fp, std::function<void (std::FILE *)> destructor) noexcept;
+	inline File(std::FILE *fp, std::function<void (std::FILE *)> destructor) noexcept
+		: m_stream(fp)
+		, m_destructor(std::move(destructor))
+	{
+		assert(m_destructor != nullptr);
+	}
 
 	/**
 	 * Closes the file.
 	 */
-	virtual ~File() noexcept;
+	virtual ~File() noexcept
+	{
+		close();
+	}
 
 	/**
 	 * Get the path.
@@ -85,6 +105,11 @@
 		return m_path;
 	}
 
+	/**
+	 * Get the handle.
+	 *
+	 * @return the handle or nullptr if the stream was closed
+	 */
 	inline std::FILE *handle() noexcept
 	{
 		return m_stream;
@@ -93,66 +118,13 @@
 	/**
 	 * Force close, can be safely called multiple times.
 	 */
-	void close() noexcept;
-
-	/**
-	 * Tells if the file was closed.
-	 *
-	 * @return true if closed
-	 */
-	bool isClosed() noexcept;
-
-	/**
-	 * std::fseek wrapper.
-	 *
-	 * @param offset the offset
-	 * @param origin the origin (SEEK_SET, *)
-	 * @throw std::runtime_error on failure
-	 */
-	void seek(long offset, long origin);
-
-	/**
-	 * std::ftell wrapper.
-	 *
-	 * @return the position
-	 * @throw std::runtime_error on failure
-	 */
-	unsigned tell();
-
-	/**
-	 * Read until the next line and discards the \\n character.
-	 *
-	 * @return the next line or empty if EOF
-	 * @throw std::runtime_error on failure
-	 */
-	std::string readline();
-
-	/**
-	 * Read the specified amount of characters.
-	 *
-	 * If amount is less than 0, the maximum is read.
-	 *
-	 * @pre amount != 0
-	 * @param amount the number of characters to read
-	 * @return the read string
-	 * @throw std::runtime_error on failure
-	 */
-	std::string read(int amount = -1);
-
-	/**
-	 * Write the string to the file.
-	 *
-	 * @param data the data to write
-	 * @throw std::runtime_error on failure
-	 */
-	void write(const std::string &data);
-
-	/**
-	 * Check if the file reached the end.
-	 *
-	 * @return true if eof
-	 */
-	bool eof() const noexcept;
+	inline void close() noexcept
+	{
+		if (m_stream) {
+			m_destructor(m_stream);
+			m_stream = nullptr;
+		}
+	}
 };
 
 namespace duk {