CGOError --> CGOErrCode

lib/srv/desktop/rdp/rdpclient/client.go

@@ -66,7 +66,6 @@ import "C"
 import (
 	"context"
 	"errors"
-	"fmt"
 	"image"
 	"io"
 	"os"
@@ -226,8 +225,8 @@ func (c *Client) connect(ctx context.Context) error {
 		C.uint16_t(c.clientHeight),
 		C.bool(c.cfg.AllowClipboard),
 	)
-	if err := cgoError(res.err); err != nil {
-		return trace.Wrap(err)
+	if res.err != C.ErrCodeSuccess {
+		return trace.ConnectionProblem(nil, "RDP connection failed")
 	}
 	c.rustClient = res.client
 	return nil
@@ -245,7 +244,7 @@ func (c *Client) start() {
 
 		// C.read_rdp_output blocks for the duration of the RDP connection and
 		// calls handle_bitmap repeatedly with the incoming bitmaps.
-		if err := cgoError(C.read_rdp_output(c.rustClient)); err != nil {
+		if err := C.read_rdp_output(c.rustClient); err != C.ErrCodeSuccess {
 			c.cfg.Log.Warningf("Failed reading RDP output frame: %v", err)
 
 			// close the TDP connection to the browser
@@ -283,16 +282,15 @@ func (c *Client) start() {
 			switch m := msg.(type) {
 			case tdp.MouseMove:
 				mouseX, mouseY = m.X, m.Y
-				if err := cgoError(C.write_rdp_pointer(
+				if err := C.write_rdp_pointer(
 					c.rustClient,
 					C.CGOMousePointerEvent{
 						x:      C.uint16_t(m.X),
 						y:      C.uint16_t(m.Y),
 						button: C.PointerButtonNone,
 						wheel:  C.PointerWheelNone,
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP mouse pointer: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.MouseButton:
@@ -308,7 +306,7 @@ func (c *Client) start() {
 				default:
 					button = C.PointerButtonNone
 				}
-				if err := cgoError(C.write_rdp_pointer(
+				if err := C.write_rdp_pointer(
 					c.rustClient,
 					C.CGOMousePointerEvent{
 						x:      C.uint16_t(mouseX),
@@ -317,8 +315,7 @@ func (c *Client) start() {
 						down:   m.State == tdp.ButtonPressed,
 						wheel:  C.PointerWheelNone,
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP mouse button: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.MouseWheel:
@@ -337,7 +334,7 @@ func (c *Client) start() {
 				default:
 					wheel = C.PointerWheelNone
 				}
-				if err := cgoError(C.write_rdp_pointer(
+				if err := C.write_rdp_pointer(
 					c.rustClient,
 					C.CGOMousePointerEvent{
 						x:           C.uint16_t(mouseX),
@@ -346,29 +343,26 @@ func (c *Client) start() {
 						wheel:       uint32(wheel),
 						wheel_delta: C.int16_t(m.Delta),
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP mouse wheel: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.KeyboardButton:
-				if err := cgoError(C.write_rdp_keyboard(
+				if err := C.write_rdp_keyboard(
 					c.rustClient,
 					C.CGOKeyboardEvent{
 						code: C.uint16_t(m.KeyCode),
 						down: m.State == tdp.ButtonPressed,
 					},
-				)); err != nil {
-					c.cfg.Log.Warningf("Failed forwarding RDP key press: %v", err)
+				); err != C.ErrCodeSuccess {
 					return
 				}
 			case tdp.ClipboardData:
 				if len(m) > 0 {
-					if err := cgoError(C.update_clipboard(
+					if err := C.update_clipboard(
 						c.rustClient,
 						(*C.uint8_t)(unsafe.Pointer(&m[0])),
 						C.uint32_t(len(m)),
-					)); err != nil {
-						c.cfg.Log.Warningf("Failed forwarding RDP clipboard data: %v", err)
+					); err != C.ErrCodeSuccess {
 						return
 					}
 				} else {
@@ -382,11 +376,11 @@ func (c *Client) start() {
 }
 
 //export handle_bitmap
-func handle_bitmap(handle C.uintptr_t, cb *C.CGOBitmap) C.CGOError {
+func handle_bitmap(handle C.uintptr_t, cb *C.CGOBitmap) C.CGOErrCode {
 	return cgo.Handle(handle).Value().(*Client).handleBitmap(cb)
 }
 
-func (c *Client) handleBitmap(cb *C.CGOBitmap) C.CGOError {
+func (c *Client) handleBitmap(cb *C.CGOBitmap) C.CGOErrCode {
 	// Notify the input forwarding goroutine that we're ready for input.
 	// Input can only be sent after connection was established, which we infer
 	// from the fact that a bitmap was sent.
@@ -415,26 +409,28 @@ func (c *Client) handleBitmap(cb *C.CGOBitmap) C.CGOError {
 	copy(img.Pix, data)
 
 	if err := c.cfg.Conn.OutputMessage(tdp.NewPNG(img, c.cfg.Encoder)); err != nil {
-		return C.CString(fmt.Sprintf("failed to send PNG frame %v: %v", img.Rect, err))
+		c.cfg.Log.Errorf("failed to send PNG frame %v: %v", img.Rect, err)
+		return C.ErrCodeFailure
 	}
-	return nil
+	return C.ErrCodeSuccess
 }
 
 //export handle_remote_copy
-func handle_remote_copy(handle C.uintptr_t, data *C.uint8_t, length C.uint32_t) C.CGOError {
+func handle_remote_copy(handle C.uintptr_t, data *C.uint8_t, length C.uint32_t) C.CGOErrCode {
 	goData := C.GoBytes(unsafe.Pointer(data), C.int(length))
 	return cgo.Handle(handle).Value().(*Client).handleRemoteCopy(goData)
 }
 
 // handleRemoteCopy is called from Rust when data is copied
 // on the remote desktop
-func (c *Client) handleRemoteCopy(data []byte) C.CGOError {
+func (c *Client) handleRemoteCopy(data []byte) C.CGOErrCode {
 	c.cfg.Log.Debugf("Received %d bytes of clipboard data from Windows desktop", len(data))
 
 	if err := c.cfg.Conn.OutputMessage(tdp.ClipboardData(data)); err != nil {
-		return C.CString(fmt.Sprintf("failed to send clipboard data: %v", err))
+		c.cfg.Log.Errorf("failed handling remote copy: %v", err)
+		return C.ErrCodeFailure
 	}
-	return nil
+	return C.ErrCodeSuccess
 }
 
 // Wait blocks until the client disconnects and runs the cleanup.
@@ -452,8 +448,8 @@ func (c *Client) Close() {
 	c.closeOnce.Do(func() {
 		c.handle.Delete()
 
-		if err := cgoError(C.close_rdp(c.rustClient)); err != nil {
-			c.cfg.Log.Warningf("Error closing RDP connection: %v", err)
+		if err := C.close_rdp(c.rustClient); err != C.ErrCodeSuccess {
+			c.cfg.Log.Warningf("failed to close the RDP client")
 		}
 	})
 }
@@ -473,19 +469,3 @@ func (c *Client) UpdateClientActivity() {
 	c.clientLastActive = time.Now().UTC()
 	c.clientActivityMu.Unlock()
 }
-
-// cgoError converts from a CGO-originated error to a Go error, copying the
-// error string and releasing the CGO data.
-func cgoError(s C.CGOError) error {
-	if s == nil {
-		return nil
-	}
-	gs := C.GoString(s)
-	C.free_rust_string(s)
-	return errors.New(gs)
-}
-
-//export free_go_string
-func free_go_string(s *C.char) {
-	C.free(unsafe.Pointer(s))
-}

lib/srv/desktop/rdp/rdpclient/librdprs.h

@@ -20,6 +20,11 @@
  */
 #define CHANNEL_CHUNK_LEGNTH 1600
 
+typedef enum CGOErrCode {
+  ErrCodeSuccess = 0,
+  ErrCodeFailure = 1,
+} CGOErrCode;
+
 typedef enum CGOPointerButton {
   PointerButtonNone,
   PointerButtonLeft,
@@ -46,14 +51,9 @@ typedef enum CGOPointerWheel {
  */
 typedef struct Client Client;
 
-/**
- * CGOError is an alias for a C string pointer, for C API clarity.
- */
-typedef char *CGOError;
-
 typedef struct ClientOrError {
   struct Client *client;
-  CGOError err;
+  enum CGOErrCode err;
 } ClientOrError;
 
 /**
@@ -126,7 +126,7 @@ struct ClientOrError connect_rdp(uintptr_t go_ref,
  *
  * `client_ptr` must be a valid pointer to a Client.
  */
-CGOError update_clipboard(struct Client *client_ptr, uint8_t *data, uint32_t len);
+enum CGOErrCode update_clipboard(struct Client *client_ptr, uint8_t *data, uint32_t len);
 
 /**
  * `read_rdp_output` reads incoming RDP bitmap frames from client at client_ref and forwards them to
@@ -137,28 +137,28 @@ CGOError update_clipboard(struct Client *client_ptr, uint8_t *data, uint32_t len
  * `client_ptr` must be a valid pointer to a Client.
  * `handle_bitmap` *must not* free the memory of CGOBitmap.
  */
-CGOError read_rdp_output(struct Client *client_ptr);
+enum CGOErrCode read_rdp_output(struct Client *client_ptr);
 
 /**
  * # Safety
  *
  * client_ptr must be a valid pointer to a Client.
  */
-CGOError write_rdp_pointer(struct Client *client_ptr, struct CGOMousePointerEvent pointer);
+enum CGOErrCode write_rdp_pointer(struct Client *client_ptr, struct CGOMousePointerEvent pointer);
 
 /**
  * # Safety
  *
  * client_ptr must be a valid pointer to a Client.
  */
-CGOError write_rdp_keyboard(struct Client *client_ptr, struct CGOKeyboardEvent key);
+enum CGOErrCode write_rdp_keyboard(struct Client *client_ptr, struct CGOKeyboardEvent key);
 
 /**
  * # Safety
  *
  * client_ptr must be a valid pointer to a Client.
  */
-CGOError close_rdp(struct Client *client_ptr);
+enum CGOErrCode close_rdp(struct Client *client_ptr);
 
 /**
  * free_rdp lets the Go side inform us when it's done with Client and it can be dropped.
@@ -176,8 +176,6 @@ void free_rdp(struct Client *client_ptr);
  */
 void free_rust_string(char *s);
 
-extern void free_go_string(char *s);
-
-extern CGOError handle_bitmap(uintptr_t client_ref, struct CGOBitmap *b);
+extern enum CGOErrCode handle_bitmap(uintptr_t client_ref, struct CGOBitmap *b);
 
-extern CGOError handle_remote_copy(uintptr_t client_ref, uint8_t *data, uint32_t len);
+extern enum CGOErrCode handle_remote_copy(uintptr_t client_ref, uint8_t *data, uint32_t len);

lib/srv/desktop/rdp/rdpclient/src/cliprdr.rs

@@ -30,18 +30,18 @@ pub const CHANNEL_NAME: &str = "cliprdr";
 /// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpeclip/fb9b7e0b-6db4-41c2-b83c-f889c1ee7688
 pub struct Client {
     clipboard: HashMap<u32, Vec<u8>>,
-    on_remote_copy: Box<dyn Fn(Vec<u8>)>,
+    on_remote_copy: Box<dyn Fn(Vec<u8>) -> RdpResult<()>>,
     vchan: vchan::Client,
 }
 
 impl Default for Client {
     fn default() -> Self {
-        Self::new(Box::new(|_| {}))
+        Self::new(Box::new(|_| Ok(())))
     }
 }
 
 impl Client {
-    pub fn new(on_remote_copy: Box<dyn Fn(Vec<u8>)>) -> Self {
+    pub fn new(on_remote_copy: Box<dyn Fn(Vec<u8>) -> RdpResult<()>>) -> Self {
         Client {
             clipboard: HashMap::new(),
             on_remote_copy,
@@ -284,7 +284,7 @@ impl Client {
             resp.data.truncate(resp.data.len() - 1);
         }
 
-        (self.on_remote_copy)(resp.data);
+        (self.on_remote_copy)(resp.data)?;
 
         Ok(vec![])
     }
@@ -1015,6 +1015,7 @@ mod tests {
 
         let mut c = Client::new(Box::new(move |vec| {
             send.send(vec).unwrap();
+            Ok(())
         }));
 
         let data_resp = FormatDataResponsePDU {

lib/srv/desktop/rdp/rdpclient/src/errors.rs

@@ -19,6 +19,10 @@ pub fn invalid_data_error(msg: &str) -> Error {
     Error::RdpError(RdpError::new(RdpErrorKind::InvalidData, msg))
 }
 
+pub fn try_error(msg: &str) -> Error {
+    Error::TryError(msg.to_string())
+}
+
 // NTSTATUS_OK is a Windows NTStatus value that means "success".
 pub const NTSTATUS_OK: u32 = 0;
 // SPECIAL_NO_RESPONSE is our custom (not defined by Windows) NTStatus value that means "don't send