Merge fix for cherry-pick of df12049f

Firmware/fibre-cpp/fibre.cpp

@@ -165,9 +165,9 @@ Domain* Context::create_domain(std::string specs) {
             FIBRE_LOG(W) << "transport layer \"" << name << "\" not implemented";
         } else {
             domain->channel_discovery_handles[name] = nullptr;
-            it->second->start_channel_discovery(&*colon_end, next_delim - colon_end,
-                    &domain->channel_discovery_handles[name],
-                    MEMBER_CB(domain, on_found_channels));
+            it->second->start_channel_discovery(domain,
+                    &*colon_end, next_delim - colon_end,
+                    &domain->channel_discovery_handles[name]);
         }
 
         prev_delim = std::min(next_delim + 1, specs.end());
@@ -225,7 +225,7 @@ void Domain::stop_discovery() {
 }
 #endif
 
-void Domain::on_found_channels(ChannelDiscoveryResult result) {
+void Domain::add_channels(ChannelDiscoveryResult result) {
     FIBRE_LOG(D) << "found channels!";
 
     if (result.status != kFibreOk) {

Firmware/fibre-cpp/include/fibre/bufptr.hpp

@@ -34,7 +34,7 @@ struct generic_bufptr_t {
         : generic_bufptr_t(vector.data(), vector.size()) {}
 
     generic_bufptr_t(const generic_bufptr_t<typename std::remove_const<T>::type>& other)
-        : generic_bufptr_t(other.begin_, other.end_) {}
+        : generic_bufptr_t(other.begin(), other.end()) {}
 
     generic_bufptr_t& operator+=(size_t num) {
         if (!soft_assert(num <= size())) {

Firmware/fibre-cpp/include/fibre/callback.hpp

@@ -56,20 +56,20 @@ public:
         return cb_;
     }
     
-    TRet invoke(TArgs ... result) const {
+    TRet invoke(TArgs ... arg) const {
         if (cb_) {
-            return (*cb_)(ctx_, result...);
+            return (*cb_)(ctx_, arg...);
         }
         return detail::get_default<TRet>::val();
     }
 
-    TRet invoke_and_clear(TArgs ... result) {
+    TRet invoke_and_clear(TArgs ... arg) {
         void* ctx = ctx_;
         auto cb = cb_;
         ctx_ = nullptr;
         cb_ = nullptr;
         if (cb) {
-            return (*cb)(ctx, result...);
+            return (*cb)(ctx, arg...);
         }
         return detail::get_default<TRet>::val();
     }

Firmware/fibre-cpp/include/fibre/channel_discoverer.hpp

@@ -16,12 +16,16 @@ struct ChannelDiscoveryResult {
 
 struct ChannelDiscoveryContext {};
 
+class Domain; // defined in fibre.hpp
+
 class ChannelDiscoverer {
 public:
+    // TODO: maybe we should remove "handle" because a discovery can also be
+    // uniquely identified by domain.
     virtual void start_channel_discovery(
+        Domain* domain,
         const char* specs, size_t specs_len,
-        ChannelDiscoveryContext** handle,
-        Callback<void, ChannelDiscoveryResult> on_found_channels) = 0;
+        ChannelDiscoveryContext** handle) = 0;
     virtual int stop_channel_discovery(ChannelDiscoveryContext* handle) = 0;
 
 protected:

Firmware/fibre-cpp/include/fibre/fibre.hpp

@@ -100,7 +100,7 @@ public:
     void stop_discovery();
 #endif
 
-    void on_found_channels(ChannelDiscoveryResult result);
+    void add_channels(ChannelDiscoveryResult result);
 
     Context* ctx;
 private:

Firmware/fibre-cpp/include/fibre/libfibre.h

@@ -56,7 +56,6 @@ extern "C" {
 #endif
 
 struct LibFibreCtx;
-struct LibFibreChannelDiscoveryCtx;
 struct LibFibreDiscoveryCtx;
 struct LibFibreCallContext;
 struct LibFibreObject;
@@ -143,8 +142,8 @@ struct LibFibreEventLoop {
  * @param specs, specs_length: The specs string that specifies discoverer-specific
  *        filter parameters.
  */
-typedef void (*on_start_discovery_cb_t)(void* ctx, LibFibreChannelDiscoveryCtx* discovery_ctx, const char* specs, size_t specs_length);
-typedef void (*on_stop_discovery_cb_t)(void* ctx, LibFibreChannelDiscoveryCtx* discovery_ctx);
+typedef void (*on_start_discovery_cb_t)(void* ctx, LibFibreDomain* domain, const char* specs, size_t specs_length);
+typedef void (*on_stop_discovery_cb_t)(void* ctx, LibFibreDomain* domain);
 
 /**
  * @brief on_found_object callback type for libfibre_start_discovery().

Firmware/fibre-cpp/legacy_object_client.cpp

@@ -479,6 +479,12 @@ void LegacyCallContext::resume_from_protocol(EndpointOperationResult result) {
             auto app_result = callback.invoke(std::get<0>(continuation));
             if (!app_result.has_value()) {
                 return; // app will resume asynchronously
+            } else if (std::get<0>(continuation).status != kFibreOk) {
+                if (app_result->status != kFibreClosed || app_result->rx_buf.size() || app_result->tx_buf.size()) {
+                    FIBRE_LOG(W) << "app tried to continue a closed call";
+                }
+                FIBRE_LOG(T) << "closing call";
+                return;
             } else {
                 res = *app_result;
             }
@@ -491,6 +497,7 @@ void LegacyCallContext::resume_from_protocol(EndpointOperationResult result) {
             return; // protocol will return asynchronously
         } else {
             callback.invoke({kFibreInternalError, app_tx_end_, app_rx_buf_.begin()});
+            return;
         }
     }
 }
@@ -574,7 +581,7 @@ std::variant<LegacyCallContext::ContinueWithApp, LegacyCallContext::ContinueWith
             return ContinueWithApp{kFibreHostUnreachable, app_tx_end_, app_rx_buf_.begin()};
         } else if (result_from_protocol.status != kStreamOk) {
             FIBRE_LOG(W) << "protocol failed with " << result_from_protocol.status << " - propagating error to application";
-            return ContinueWithApp{kFibreInternalError, app_tx_end_, app_rx_buf_.begin()};
+            return ContinueWithApp{kFibreHostUnreachable, app_tx_end_, app_rx_buf_.begin()};
         }
 
         tx_pos_ = result_from_protocol.tx_end - tx_buf_.data();
@@ -590,7 +597,7 @@ std::variant<LegacyCallContext::ContinueWithApp, LegacyCallContext::ContinueWith
 
         ResultFromApp result_from_app = std::get<0>(continue_from);
 
-        if (result_from_app.status) {
+        if (result_from_app.status != kFibreOk && result_from_app.status != kFibreClosed) {
             FIBRE_LOG(W) << "application failed with " << result_from_app.status << " - dropping this call";
             return InternalError{};
         }
@@ -653,6 +660,8 @@ std::variant<LegacyCallContext::ContinueWithApp, LegacyCallContext::ContinueWith
 
         rx_buf_ = transcoded;
         rx_pos_ = 0;
+
+        FIBRE_LOG(T) << "rx buf is " << as_hex(cbufptr_t{rx_buf_});
     }
 
     progress++;

Firmware/fibre-cpp/legacy_protocol.cpp

@@ -523,6 +523,10 @@ void LegacyProtocolPacketBased::on_rx_closed(StreamStatus status) {
 }
 
 void LegacyProtocolPacketBased::on_rx_tx_closed(StreamStatus status) {
+    if (status == kStreamClosed || status == kStreamCancelled) {
+        // TODO: handle app-initiated cancellation via cancel_endpoint_operation() (currently unused)
+        status = kStreamError;
+    }
 
 #if FIBRE_ENABLE_CLIENT
     // Cancel pending endpoint operation

Firmware/fibre-cpp/libfibre.cpp

@@ -10,7 +10,53 @@
 DEFINE_LOG_TOPIC(LIBFIBRE);
 USE_LOG_TOPIC(LIBFIBRE);
 
-static const struct LibFibreVersion libfibre_version = { 0, 1, 2 };
+
+struct LibFibreChannelDiscoveryCtx {
+    fibre::Domain* domain;
+};
+
+LibFibreFunction* to_c(fibre::Function* ptr) {
+    return reinterpret_cast<LibFibreFunction*>(ptr);
+}
+fibre::Function* from_c(LibFibreFunction* ptr) {
+    return reinterpret_cast<fibre::Function*>(ptr);
+}
+void** from_c(LibFibreCallContext** ptr) {
+    return reinterpret_cast<void**>(ptr);
+}
+LibFibreDomain* to_c(fibre::Domain* ptr) {
+    return reinterpret_cast<LibFibreDomain*>(ptr);
+}
+fibre::Domain* from_c(LibFibreDomain* ptr) {
+    return reinterpret_cast<fibre::Domain*>(ptr);
+}
+LibFibreObject* to_c(fibre::Object* ptr) {
+    return reinterpret_cast<LibFibreObject*>(ptr);
+}
+fibre::Object* from_c(LibFibreObject* ptr) {
+    return reinterpret_cast<fibre::Object*>(ptr);
+}
+LibFibreInterface* to_c(fibre::Interface* ptr) {
+    return reinterpret_cast<LibFibreInterface*>(ptr);
+}
+fibre::Interface* from_c(LibFibreInterface* ptr) {
+    return reinterpret_cast<fibre::Interface*>(ptr);
+}
+LibFibreStatus to_c(fibre::Status status) {
+    return static_cast<LibFibreStatus>(status);
+}
+fibre::Status from_c(LibFibreStatus status) {
+    return static_cast<fibre::Status>(status);
+}
+LibFibreChannelDiscoveryCtx* to_c(fibre::ChannelDiscoveryContext* ptr) {
+    return reinterpret_cast<LibFibreChannelDiscoveryCtx*>(ptr);
+}
+fibre::ChannelDiscoveryContext* from_c(LibFibreChannelDiscoveryCtx* ptr) {
+    return reinterpret_cast<fibre::ChannelDiscoveryContext*>(ptr);
+}
+
+
+static const struct LibFibreVersion libfibre_version = { 0, 1, 3 };
 
 class FIBRE_PRIVATE ExternalEventLoop final : public fibre::EventLoop {
 public:
@@ -45,9 +91,9 @@ private:
 
 class ExternalDiscoverer : public fibre::ChannelDiscoverer {
     void start_channel_discovery(
+        fibre::Domain* domain,
         const char* specs, size_t specs_len,
-        fibre::ChannelDiscoveryContext** handle,
-        fibre::Callback<void, fibre::ChannelDiscoveryResult> on_found_channels) final;
+        fibre::ChannelDiscoveryContext** handle) final;
     int stop_channel_discovery(fibre::ChannelDiscoveryContext* handle) final;
 public:
     on_start_discovery_cb_t on_start_discovery;
@@ -55,25 +101,21 @@ public:
     void* cb_ctx;
 };
 
-struct LibFibreChannelDiscoveryCtx : fibre::ChannelDiscoveryContext {
-    fibre::Callback<void, fibre::ChannelDiscoveryResult> completer;
-};
 
-void ExternalDiscoverer::start_channel_discovery(const char* specs, size_t specs_len, fibre::ChannelDiscoveryContext** handle, fibre::Callback<void, fibre::ChannelDiscoveryResult> on_found_channels) {
+void ExternalDiscoverer::start_channel_discovery(fibre::Domain* domain, const char* specs, size_t specs_len, fibre::ChannelDiscoveryContext** handle) {
     LibFibreChannelDiscoveryCtx* ctx = new LibFibreChannelDiscoveryCtx{};
-    ctx->completer = on_found_channels;
     if (handle) {
-        *handle = ctx;
+        *handle = from_c(ctx);
     }
     if (on_start_discovery) {
-        (*on_start_discovery)(cb_ctx, ctx, specs, specs_len);
+        (*on_start_discovery)(cb_ctx, to_c(domain), specs, specs_len);
     }
 }
 
 int ExternalDiscoverer::stop_channel_discovery(fibre::ChannelDiscoveryContext* handle) {
-    LibFibreChannelDiscoveryCtx* ctx = static_cast<LibFibreChannelDiscoveryCtx*>(handle);
+    LibFibreChannelDiscoveryCtx* ctx = to_c(handle);
     if (on_stop_discovery) {
-        (*on_stop_discovery)(cb_ctx, ctx);
+        (*on_stop_discovery)(cb_ctx, to_c(ctx->domain));
     }
     delete ctx;
     return 0;
@@ -116,13 +158,11 @@ void AsyncStreamLink::cancel_write(TransferHandle transfer_handle) {
 
 void AsyncStreamLink::start_read(bufptr_t buffer, TransferHandle* handle, Callback<void, ReadResult> completer) {
     if (write_completer_) {
-        FIBRE_LOG(W) << "start_read: completing writer";
         size_t n_copy = std::min(buffer.size(), write_buf_.size());
         memcpy(buffer.begin(), write_buf_.begin(), n_copy);
         write_completer_.invoke_and_clear({kStreamOk, write_buf_.begin() + n_copy});
         completer.invoke({kStreamOk, buffer.begin() + n_copy});
     } else {
-        //FIBRE_LOG(W) << "start_read: waiting for writer";
         if (handle) {
             *handle = reinterpret_cast<uintptr_t>(this);
         }
@@ -207,39 +247,6 @@ struct LibFibreRxStream {
     void* on_closed_ctx;
 };
 
-LibFibreFunction* to_c(fibre::Function* ptr) {
-    return reinterpret_cast<LibFibreFunction*>(ptr);
-}
-fibre::Function* from_c(LibFibreFunction* ptr) {
-    return reinterpret_cast<fibre::Function*>(ptr);
-}
-void** from_c(LibFibreCallContext** ptr) {
-    return reinterpret_cast<void**>(ptr);
-}
-LibFibreDomain* to_c(fibre::Domain* ptr) {
-    return reinterpret_cast<LibFibreDomain*>(ptr);
-}
-fibre::Domain* from_c(LibFibreDomain* ptr) {
-    return reinterpret_cast<fibre::Domain*>(ptr);
-}
-LibFibreObject* to_c(fibre::Object* ptr) {
-    return reinterpret_cast<LibFibreObject*>(ptr);
-}
-fibre::Object* from_c(LibFibreObject* ptr) {
-    return reinterpret_cast<fibre::Object*>(ptr);
-}
-LibFibreInterface* to_c(fibre::Interface* ptr) {
-    return reinterpret_cast<LibFibreInterface*>(ptr);
-}
-fibre::Interface* from_c(LibFibreInterface* ptr) {
-    return reinterpret_cast<fibre::Interface*>(ptr);
-}
-LibFibreStatus to_c(fibre::Status status) {
-    return static_cast<LibFibreStatus>(status);
-}
-fibre::Status from_c(LibFibreStatus status) {
-    return static_cast<fibre::Status>(status);
-}
 
 void LibFibreDiscoveryCtx::on_found_object(fibre::Object* obj, fibre::Interface* intf) {
     if (on_found_object_) {
@@ -320,10 +327,10 @@ void libfibre_close_domain(LibFibreDomain* domain) {
 }
 
 void libfibre_add_channels(LibFibreDomain* domain, LibFibreRxStream** tx_channel, LibFibreTxStream** rx_channel, size_t mtu) {
-    fibre::AsyncStreamLink* tx_link = new fibre::AsyncStreamLink();
-    fibre::AsyncStreamLink* rx_link = new fibre::AsyncStreamLink();
-    LibFibreRxStream* tx = new LibFibreRxStream();
-    LibFibreTxStream* rx = new LibFibreTxStream();
+    fibre::AsyncStreamLink* tx_link = new fibre::AsyncStreamLink(); // libfibre => backend
+    fibre::AsyncStreamLink* rx_link = new fibre::AsyncStreamLink(); // backend => libfibre
+    LibFibreRxStream* tx = new LibFibreRxStream(); // libfibre => backend
+    LibFibreTxStream* rx = new LibFibreTxStream(); // backend => libfibre
     tx->source = tx_link;
     rx->sink = rx_link;
 
@@ -351,7 +358,7 @@ void libfibre_add_channels(LibFibreDomain* domain, LibFibreRxStream** tx_channel
     }
 
     fibre::ChannelDiscoveryResult result = {fibre::kFibreOk, rx_link, tx_link, mtu};
-    from_c(domain)->on_found_channels(result);
+    from_c(domain)->add_channels(result);
 }
 
 void libfibre_start_discovery(LibFibreDomain* domain, LibFibreDiscoveryCtx** handle,

Firmware/fibre-cpp/platform_support/libusb_transport.cpp

@@ -7,6 +7,7 @@
 #include "libusb_transport.hpp"
 #include "../logging.hpp"
 #include "../print_utils.hpp"
+#include <fibre/fibre.hpp>
 
 #include <algorithm>
 #include <string.h>
@@ -213,7 +214,7 @@ bool LibusbDiscoverer::deinit(int stage) {
  *        This callback will also be called for any matching channels that already exist when
  *        the discovery is started.
  */
-void LibusbDiscoverer::start_channel_discovery(const char* specs, size_t specs_len, ChannelDiscoveryContext** handle, Callback<void, ChannelDiscoveryResult> on_found_channels) {
+void LibusbDiscoverer::start_channel_discovery(Domain* domain, const char* specs, size_t specs_len, ChannelDiscoveryContext** handle) {
     FIBRE_LOG(D) << "starting discovery with filter \"" << std::string(specs, specs_len) << "\"";
 
     InterfaceSpecs interface_specs;
@@ -228,7 +229,7 @@ void LibusbDiscoverer::start_channel_discovery(const char* specs, size_t specs_l
 
     MyChannelDiscoveryContext* subscription = new MyChannelDiscoveryContext{};
     subscription->interface_specs = interface_specs;
-    subscription->on_found_channels = on_found_channels;
+    subscription->domain = domain;
     subscriptions_.push_back(subscription);
 
     for (auto& dev: known_devices_) {
@@ -517,7 +518,7 @@ void LibusbDiscoverer::consider_device(struct libusb_device *device, MyChannelDi
                     ep_out = nullptr;
                 }
 
-                subscription->on_found_channels.invoke({kFibreOk, ep_in, ep_out, mtu});
+                subscription->domain->add_channels({kFibreOk, ep_in, ep_out, mtu});
             }
         }
 

Firmware/fibre-cpp/platform_support/libusb_transport.hpp

@@ -32,13 +32,13 @@ public:
 
     struct MyChannelDiscoveryContext : ChannelDiscoveryContext {
         InterfaceSpecs interface_specs;
-        Callback<void, ChannelDiscoveryResult> on_found_channels;
+        Domain* domain;
     };
 
     constexpr static const char* get_name() { return "usb"; }
     bool init(EventLoop* event_loop);
     bool deinit() { return deinit(INT_MAX); }
-    void start_channel_discovery(const char* specs, size_t specs_len, ChannelDiscoveryContext** handle, Callback<void, ChannelDiscoveryResult> on_found_channels) final;
+    void start_channel_discovery(Domain* domain, const char* specs, size_t specs_len, ChannelDiscoveryContext** handle) final;
     int stop_channel_discovery(ChannelDiscoveryContext* handle) final;
 
 private:

Firmware/fibre-cpp/platform_support/posix_tcp_backend.cpp

@@ -2,6 +2,7 @@
 #include "posix_tcp_backend.hpp"
 #include "posix_socket.hpp"
 #include "../logging.hpp"
+#include <fibre/fibre.hpp>
 #include <signal.h>
 #include <unistd.h>
 #include <algorithm>
@@ -33,27 +34,27 @@ bool PosixTcpBackend::deinit() {
     return true;
 }
 
-void PosixTcpBackend::start_channel_discovery(const char* specs, size_t specs_len, ChannelDiscoveryContext** handle, Callback<void, ChannelDiscoveryResult> on_found_channels) {
+void PosixTcpBackend::start_channel_discovery(Domain* domain, const char* specs, size_t specs_len, ChannelDiscoveryContext** handle) {
     const char* address_begin;
     const char* address_end;
     int port;
 
     if (!event_loop_) {
         FIBRE_LOG(E) << "not initialized";
-        on_found_channels.invoke({kFibreInvalidArgument, nullptr, nullptr, 0});
-        return;
+        //on_found_channels.invoke({kFibreInvalidArgument, nullptr, nullptr, 0});
+        return; // TODO: error reporting
     }
 
     if (!try_parse_key(specs, specs + specs_len, "address", &address_begin, &address_end)) {
         FIBRE_LOG(E) << "no address specified";
-        on_found_channels.invoke({kFibreInvalidArgument, nullptr, nullptr, 0});
-        return;
+        //on_found_channels.invoke({kFibreInvalidArgument, nullptr, nullptr, 0});
+        return; // TODO: error reporting
     }
 
     if (!try_parse_key(specs, specs + specs_len, "port", &port)) {
         FIBRE_LOG(E) << "no port specified";
-        on_found_channels.invoke({kFibreInvalidArgument, nullptr, nullptr, 0});
-        return;
+        //on_found_channels.invoke({kFibreInvalidArgument, nullptr, nullptr, 0});
+        return; // TODO: error reporting
     }
 
     n_discoveries_++;
@@ -61,7 +62,7 @@ void PosixTcpBackend::start_channel_discovery(const char* specs, size_t specs_le
     TcpChannelDiscoveryContext* ctx = new TcpChannelDiscoveryContext(); // TODO: free
     ctx->parent = this;
     ctx->address = {{address_begin, address_end}, port};
-    ctx->on_found_channels = on_found_channels;
+    ctx->domain = domain;
     ctx->resolve_address();
 }
 
@@ -119,7 +120,7 @@ void PosixTcpBackend::TcpChannelDiscoveryContext::on_connected(std::optional<soc
     if (socket_id.has_value()) {
         auto socket = new PosixSocket{}; // TODO: free
         if (socket->init(parent->event_loop_, *socket_id)) {
-            on_found_channels.invoke({kFibreOk, socket, socket, SIZE_MAX});
+            domain->add_channels({kFibreOk, socket, socket, SIZE_MAX});
             return;
         }
         delete socket;

Firmware/fibre-cpp/platform_support/posix_tcp_backend.hpp

@@ -20,14 +20,14 @@ public:
     bool init(EventLoop* event_loop);
     bool deinit();
 
-    void start_channel_discovery(const char* specs, size_t specs_len, ChannelDiscoveryContext** handle, Callback<void, ChannelDiscoveryResult> on_found_channels) final;
+    void start_channel_discovery(Domain* domain, const char* specs, size_t specs_len, ChannelDiscoveryContext** handle) final;
     int stop_channel_discovery(ChannelDiscoveryContext* handle) final;
 
 private:
     struct TcpChannelDiscoveryContext {
         PosixTcpBackend* parent;
         std::tuple<std::string, int> address;
-        Callback<void, ChannelDiscoveryResult> on_found_channels;
+        Domain* domain;
         AddressResolutionContext* addr_resolution_ctx;
         ConnectionContext* connection_ctx;
         float lookup_period = 1.0f; // wait 1s for next address resolution