ssl: make bind & listen into void functions (they throw exceptions)

jepler · jepler · commit 8c5d9d28d8bb · 2024-03-21T14:06:32.000-05:00
diff --git a/shared-bindings/ssl/SSLSocket.c b/shared-bindings/ssl/SSLSocket.c
@@ -88,11 +88,7 @@ STATIC mp_obj_t ssl_sslsocket_bind(mp_obj_t self_in, mp_obj_t addr_in) {
     mp_obj_t *addr_items;
     mp_obj_get_array_fixed_n(addr_in, 2, &addr_items);
 
-    size_t error = common_hal_ssl_sslsocket_bind(self, addr_in);
-    if (error != 0) {
-        mp_raise_OSError(error);
-    }
-
+    common_hal_ssl_sslsocket_bind(self, addr_in);
     return mp_const_none;
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_2(ssl_sslsocket_bind_obj, ssl_sslsocket_bind);
diff --git a/shared-bindings/ssl/SSLSocket.h b/shared-bindings/ssl/SSLSocket.h
@@ -35,12 +35,12 @@
 extern const mp_obj_type_t ssl_sslsocket_type;
 
 mp_obj_t common_hal_ssl_sslsocket_accept(ssl_sslsocket_obj_t *self);
-size_t common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr);
+void common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr);
 void common_hal_ssl_sslsocket_close(ssl_sslsocket_obj_t *self);
 void common_hal_ssl_sslsocket_connect(ssl_sslsocket_obj_t *self, mp_obj_t addr);
 bool common_hal_ssl_sslsocket_get_closed(ssl_sslsocket_obj_t *self);
 bool common_hal_ssl_sslsocket_get_connected(ssl_sslsocket_obj_t *self);
-bool common_hal_ssl_sslsocket_listen(ssl_sslsocket_obj_t *self, int backlog);
+void common_hal_ssl_sslsocket_listen(ssl_sslsocket_obj_t *self, int backlog);
 mp_uint_t common_hal_ssl_sslsocket_recv_into(ssl_sslsocket_obj_t *self, uint8_t *buf, uint32_t len);
 mp_uint_t common_hal_ssl_sslsocket_send(ssl_sslsocket_obj_t *self, const uint8_t *buf, uint32_t len);
 void common_hal_ssl_sslsocket_settimeout(ssl_sslsocket_obj_t *self, mp_obj_t timeout_obj);
diff --git a/shared-module/ssl/SSLSocket.c b/shared-module/ssl/SSLSocket.c
@@ -106,6 +106,10 @@ STATIC NORETURN void mbedtls_raise_error(int err) {
     #endif
 }
 
+// Because ssl_socket_send and ssl_socket_recv_into are callbacks from mbedtls code,
+// it is not OK to exit them by raising an exception (nlr_jump'ing through
+// foreign code is not permitted). Instead, preserve the error number of any OSError
+// and turn anything else into -MP_EINVAL.
 STATIC int call_method_errno(size_t n_args, const mp_obj_t *args) {
     nlr_buf_t nlr;
     mp_int_t result = -MP_EINVAL;
@@ -140,28 +144,28 @@ static int ssl_socket_recv_into(ssl_sslsocket_obj_t *self, byte *buf, size_t len
     return call_method_errno(1, self->recv_into_args);
 }
 
-static int ssl_socket_connect(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {
+static void ssl_socket_connect(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {
     self->connect_args[2] = addr_in;
-    return call_method_errno(1, self->connect_args);
+    mp_call_method_n_kw(1, 0, self->connect_args);
 }
 
-static int ssl_socket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {
+static void ssl_socket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {
     self->bind_args[2] = addr_in;
-    return call_method_errno(1, self->bind_args);
+    mp_call_method_n_kw(1, 0, self->bind_args);
 }
 
-static int ssl_socket_close(ssl_sslsocket_obj_t *self) {
-    return call_method_errno(0, self->close_args);
+static void ssl_socket_close(ssl_sslsocket_obj_t *self) {
+    mp_call_method_n_kw(0, 0, self->close_args);
 }
 
 static void ssl_socket_settimeout(ssl_sslsocket_obj_t *self, mp_obj_t timeout_obj) {
     self->settimeout_args[2] = timeout_obj;
-    return mp_call_method_n_kw(1, 0, self->settimeout_args);
+    mp_call_method_n_kw(1, 0, self->settimeout_args);
 }
 
-static int ssl_socket_listen(ssl_sslsocket_obj_t *self, mp_int_t backlog) {
+static void ssl_socket_listen(ssl_sslsocket_obj_t *self, mp_int_t backlog) {
     self->listen_args[2] = MP_OBJ_NEW_SMALL_INT(backlog);
-    return call_method_errno(1, self->listen_args);
+    mp_call_method_n_kw(1, 0, self->listen_args);
 }
 
 static mp_obj_t ssl_socket_accept(ssl_sslsocket_obj_t *self) {
@@ -179,13 +183,10 @@ STATIC int _mbedtls_ssl_send(void *ctx, const byte *buf, size_t len) {
         if (mp_is_nonblocking_error(err)) {
             return MBEDTLS_ERR_SSL_WANT_WRITE;
         }
-        return -err; // convert an MP_ERRNO to something mbedtls passes through as error
-    } else {
-        return out_sz;
     }
+    return out_sz;
 }
 
-// _mbedtls_ssl_recv is called by mbedtls to receive bytes from the underlying socket
 STATIC int _mbedtls_ssl_recv(void *ctx, byte *buf, size_t len) {
     ssl_sslsocket_obj_t *self = (ssl_sslsocket_obj_t *)ctx;
 
@@ -196,10 +197,8 @@ STATIC int _mbedtls_ssl_recv(void *ctx, byte *buf, size_t len) {
         if (mp_is_nonblocking_error(err)) {
             return MBEDTLS_ERR_SSL_WANT_READ;
         }
-        return -err;
-    } else {
-        return out_sz;
     }
+    return out_sz;
 }
 
 
@@ -361,8 +360,8 @@ mp_uint_t common_hal_ssl_sslsocket_send(ssl_sslsocket_obj_t *self, const uint8_t
     mbedtls_raise_error(ret);
 }
 
-size_t common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {
-    return ssl_socket_bind(self, addr_in);
+void common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {
+    ssl_socket_bind(self, addr_in);
 }
 
 void common_hal_ssl_sslsocket_close(ssl_sslsocket_obj_t *self) {
@@ -426,7 +425,7 @@ bool common_hal_ssl_sslsocket_get_connected(ssl_sslsocket_obj_t *self) {
     return !self->closed;
 }
 
-bool common_hal_ssl_sslsocket_listen(ssl_sslsocket_obj_t *self, int backlog) {
+void common_hal_ssl_sslsocket_listen(ssl_sslsocket_obj_t *self, int backlog) {
     return ssl_socket_listen(self, backlog);
 }
 

Original file line number	Diff line number	Diff line change
`@@ -106,6 +106,10 @@ STATIC NORETURN void mbedtls_raise_error(int err) {`
`106`	`106`	`#endif`
`107`	`107`	`}`
`108`	`108`
	`109`	`+// Because ssl_socket_send and ssl_socket_recv_into are callbacks from mbedtls code,`
	`110`	`+// it is not OK to exit them by raising an exception (nlr_jump'ing through`
	`111`	`+// foreign code is not permitted). Instead, preserve the error number of any OSError`
	`112`	`+// and turn anything else into -MP_EINVAL.`
`109`	`113`	`STATIC int call_method_errno(size_t n_args, const mp_obj_t *args) {`
`110`	`114`	`nlr_buf_t nlr;`
`111`	`115`	`mp_int_t result = -MP_EINVAL;`
`@@ -140,28 +144,28 @@ static int ssl_socket_recv_into(ssl_sslsocket_obj_t self, byte buf, size_t len`
`140`	`144`	`return call_method_errno(1, self->recv_into_args);`
`141`	`145`	`}`
`142`	`146`
`143`		`-static int ssl_socket_connect(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {`
	`147`	`+static void ssl_socket_connect(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {`
`144`	`148`	`self->connect_args[2] = addr_in;`
`145`		`- return call_method_errno(1, self->connect_args);`
	`149`	`+ mp_call_method_n_kw(1, 0, self->connect_args);`
`146`	`150`	`}`
`147`	`151`
`148`		`-static int ssl_socket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {`
	`152`	`+static void ssl_socket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {`
`149`	`153`	`self->bind_args[2] = addr_in;`
`150`		`- return call_method_errno(1, self->bind_args);`
	`154`	`+ mp_call_method_n_kw(1, 0, self->bind_args);`
`151`	`155`	`}`
`152`	`156`
`153`		`-static int ssl_socket_close(ssl_sslsocket_obj_t *self) {`
`154`		`- return call_method_errno(0, self->close_args);`
	`157`	`+static void ssl_socket_close(ssl_sslsocket_obj_t *self) {`
	`158`	`+ mp_call_method_n_kw(0, 0, self->close_args);`
`155`	`159`	`}`
`156`	`160`
`157`	`161`	`static void ssl_socket_settimeout(ssl_sslsocket_obj_t *self, mp_obj_t timeout_obj) {`
`158`	`162`	`self->settimeout_args[2] = timeout_obj;`
`159`		`- return mp_call_method_n_kw(1, 0, self->settimeout_args);`
	`163`	`+ mp_call_method_n_kw(1, 0, self->settimeout_args);`
`160`	`164`	`}`
`161`	`165`
`162`		`-static int ssl_socket_listen(ssl_sslsocket_obj_t *self, mp_int_t backlog) {`
	`166`	`+static void ssl_socket_listen(ssl_sslsocket_obj_t *self, mp_int_t backlog) {`
`163`	`167`	`self->listen_args[2] = MP_OBJ_NEW_SMALL_INT(backlog);`
`164`		`- return call_method_errno(1, self->listen_args);`
	`168`	`+ mp_call_method_n_kw(1, 0, self->listen_args);`
`165`	`169`	`}`
`166`	`170`
`167`	`171`	`static mp_obj_t ssl_socket_accept(ssl_sslsocket_obj_t *self) {`
`@@ -179,13 +183,10 @@ STATIC int _mbedtls_ssl_send(void ctx, const byte buf, size_t len) {`
`179`	`183`	`if (mp_is_nonblocking_error(err)) {`
`180`	`184`	`return MBEDTLS_ERR_SSL_WANT_WRITE;`
`181`	`185`	`}`
`182`		`- return -err; // convert an MP_ERRNO to something mbedtls passes through as error`
`183`		`- } else {`
`184`		`- return out_sz;`
`185`	`186`	`}`
	`187`	`+ return out_sz;`
`186`	`188`	`}`
`187`	`189`
`188`		`-// _mbedtls_ssl_recv is called by mbedtls to receive bytes from the underlying socket`
`189`	`190`	`STATIC int _mbedtls_ssl_recv(void ctx, byte buf, size_t len) {`
`190`	`191`	`ssl_sslsocket_obj_t self = (ssl_sslsocket_obj_t )ctx;`
`191`	`192`
`@@ -196,10 +197,8 @@ STATIC int _mbedtls_ssl_recv(void ctx, byte buf, size_t len) {`
`196`	`197`	`if (mp_is_nonblocking_error(err)) {`
`197`	`198`	`return MBEDTLS_ERR_SSL_WANT_READ;`
`198`	`199`	`}`
`199`		`- return -err;`
`200`		`- } else {`
`201`		`- return out_sz;`
`202`	`200`	`}`
	`201`	`+ return out_sz;`
`203`	`202`	`}`
`204`	`203`
`205`	`204`
`@@ -361,8 +360,8 @@ mp_uint_t common_hal_ssl_sslsocket_send(ssl_sslsocket_obj_t *self, const uint8_t`
`361`	`360`	`mbedtls_raise_error(ret);`
`362`	`361`	`}`
`363`	`362`
`364`		`-size_t common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {`
`365`		`- return ssl_socket_bind(self, addr_in);`
	`363`	`+void common_hal_ssl_sslsocket_bind(ssl_sslsocket_obj_t *self, mp_obj_t addr_in) {`
	`364`	`+ ssl_socket_bind(self, addr_in);`
`366`	`365`	`}`
`367`	`366`
`368`	`367`	`void common_hal_ssl_sslsocket_close(ssl_sslsocket_obj_t *self) {`
`@@ -426,7 +425,7 @@ bool common_hal_ssl_sslsocket_get_connected(ssl_sslsocket_obj_t *self) {`
`426`	`425`	`return !self->closed;`
`427`	`426`	`}`
`428`	`427`
`429`		`-bool common_hal_ssl_sslsocket_listen(ssl_sslsocket_obj_t *self, int backlog) {`
	`428`	`+void common_hal_ssl_sslsocket_listen(ssl_sslsocket_obj_t *self, int backlog) {`
`430`	`429`	`return ssl_socket_listen(self, backlog);`
`431`	`430`	`}`
`432`	`431`