fd: deprecate Resource in favor of NamedPtr

Rewrite Fd using a much simpler named resource template class with
a more straightforward derivation strategy.

Behavior change:  we no longer throw an exception while calling get_fd()
on a closed Fd.  This does not seem to bother any current callers except
for the tests.

Signed-off-by: Zygo Blaxell <bees@furryterror.org>

include/crucible/fd.h

@@ -1,7 +1,7 @@
 #ifndef CRUCIBLE_FD_H
 #define CRUCIBLE_FD_H
 
-#include "crucible/resource.h"
+#include "crucible/namedptr.h"
 
 #include <cstring>
 
@@ -34,29 +34,28 @@ namespace crucible {
 		IOHandle(IOHandle &&) = delete;
 		IOHandle& operator=(IOHandle &&) = delete;
 		IOHandle& operator=(const IOHandle &) = delete;
-	protected:
 		int	m_fd;
-		IOHandle& operator=(int that) { m_fd = that; return *this; }
 	public:
 		virtual ~IOHandle();
-		IOHandle(int fd);
+		IOHandle(int fd = -1);
-		IOHandle();
+		int get_fd() const;
-
 		void close();
-		int get_fd() const { return m_fd; }
-		int release_fd();
 	};
 
-        template <>
+	class Fd {
-        struct ResourceTraits<int, IOHandle> {
+		static NamedPtr<IOHandle, int> s_named_ptr;
-                int get_key(const IOHandle &res) const { return res.get_fd(); }
+		shared_ptr<IOHandle> m_handle;
-                shared_ptr<IOHandle> make_resource(int fd) const { return make_shared<IOHandle>(fd); }
+	public:
-                bool is_null_key(const int &key) const { return key < 0; }
+		using resource_type = IOHandle;
-                int get_null_key() const { return -1; }
+		Fd();
+		Fd(int fd);
+		Fd &operator=(int fd);
+		Fd &operator=(const shared_ptr<IOHandle> &);
+		operator int() const;
+		bool operator!() const;
+		shared_ptr<IOHandle> operator->() const;
 	};
 
-        typedef ResourceHandle<int, IOHandle> Fd;
-
 	void set_relative_path(string path);
 	string relative_path();
 

lib/fd.cc

@@ -107,12 +107,6 @@ namespace crucible {
 		}
 	}
 
-	IOHandle::IOHandle() :
-		m_fd(-1)
-	{
-		CHATTER_TRACE("open fd " << m_fd << " in " << this);
-	}
-
 	IOHandle::IOHandle(int fd) :
 		m_fd(fd)
 	{
@@ -120,12 +114,52 @@ namespace crucible {
 	}
 
 	int
-	IOHandle::release_fd()
+	IOHandle::get_fd() const
 	{
-		CHATTER_TRACE("release fd " << m_fd << " in " << this);
+		return m_fd;
-		int rv = m_fd;
+	}
-		m_fd = -1;
+
-		return rv;
+	NamedPtr<IOHandle, int> Fd::s_named_ptr([](int fd) { return make_shared<IOHandle>(fd); });
+
+	Fd::Fd() :
+		m_handle(s_named_ptr(-1))
+	{
+	}
+
+	Fd::Fd(int fd) :
+		m_handle(s_named_ptr(fd < 0 ? -1 : fd))
+	{
+	}
+
+	Fd &
+	Fd::operator=(int const fd)
+	{
+		m_handle = s_named_ptr(fd < 0 ? -1 : fd);
+		return *this;
+	}
+
+	Fd &
+	Fd::operator=(const shared_ptr<IOHandle> &handle)
+	{
+		m_handle = s_named_ptr.insert(handle, handle->get_fd());
+		return *this;
+	}
+
+	Fd::operator int() const
+	{
+		return m_handle->get_fd();
+	}
+
+	bool
+	Fd::operator!() const
+	{
+		return m_handle->get_fd() < 0;
+	}
+
+	shared_ptr<IOHandle>
+	Fd::operator->() const
+	{
+		return m_handle;
 	}
 
 	// XXX: necessary?  useful?

test/fd.cc

@@ -207,8 +207,8 @@ static void test_assign_int_close()
 		assert(j == -1);
 		// Bonus conversion operator tests
 		assert(fd == -1);
-		// Chasing a closed ref now triggers an exception
+		// Chasing a closed ref no longer triggers an exception
-		assert(catch_all([&]() { return fd->get_fd() == -1; }));
+		assert(fd->get_fd() == -1);
 	}
 	assert_is_closed(i, true);
 }
@@ -228,8 +228,8 @@ static void test_assign_int_close_2()
 		assert(j == -1);
 		// Bonus conversion operator tests
 		assert(fd == -1);
-		// Chasing a closed ref now triggers an exception
+		// Chasing a closed ref no longer triggers an exception
-		assert(catch_all([&]() { return fd->get_fd() == -1; }));
+		assert(fd->get_fd() == -1);
 	}
 	assert_is_closed(i, true);
 }
@@ -285,13 +285,23 @@ static void test_shared_close_method()
 
 struct DerivedFdResource : public Fd::resource_type {
 	string	m_name;
-	DerivedFdResource(string name) : m_name(name) {
+	DerivedFdResource(string name) : Fd::resource_type(open(name.c_str(), O_RDONLY)), m_name(name) {
-		Fd::resource_type::operator=(open(name.c_str(), O_RDONLY));
 		assert_is_closed(this->get_fd(), false);
 	}
 	const string &name() const { return m_name; }
 };
 
+template<class T>
+shared_ptr<T>
+cast(const Fd &fd)
+{
+	auto dp = dynamic_pointer_cast<T>(fd.operator->());
+	if (!dp) {
+		throw bad_cast();
+	}
+	return dp;
+}
+
 struct DerivedFd : public Fd {
 	using resource_type = DerivedFdResource;
 	DerivedFd(string name) {
@@ -299,7 +309,7 @@ struct DerivedFd : public Fd {
 		Fd::operator=(static_pointer_cast<Fd::resource_type>(ptr));
 	}
 	shared_ptr<DerivedFdResource> operator->() const {
-		shared_ptr<DerivedFdResource> rv = cast<DerivedFdResource>();
+		shared_ptr<DerivedFdResource> rv = cast<DerivedFdResource>(*this);
 		THROW_CHECK1(out_of_range, rv, rv);
 		return rv;
 	}
@@ -328,12 +338,12 @@ static void test_derived_cast()
 	Fd fd2(fd);
 	Fd fd3 = open("fd.cc", O_RDONLY);
 	assert(fd->name() == "fd.cc");
-	assert(fd.cast<Fd::resource_type>());
+	assert(cast<Fd::resource_type>(fd));
-	assert(fd.cast<DerivedFd::resource_type>());
+	assert(cast<DerivedFd::resource_type>(fd));
-	assert(fd2.cast<Fd::resource_type>());
+	assert(cast<Fd::resource_type>(fd2));
-	assert(fd2.cast<DerivedFd::resource_type>());
+	assert(cast<DerivedFd::resource_type>(fd2));
-	assert(fd3.cast<Fd::resource_type>());
+	assert(cast<Fd::resource_type>(fd3));
-	assert(catch_all([&](){ assert(!fd3.cast<DerivedFd::resource_type>()); } ));
+	assert(catch_all([&](){ assert(!cast<DerivedFd::resource_type>(fd3)); } ));
 }
 
 static void test_derived_map()