Browse Source

Clean up handle_ops and PAL_HANDLE usage

Michał Kowalczyk 4 years ago
parent
commit
a6e1524e6e

+ 1 - 1
LibOS/shim/src/ipc/shim_ipc_helper.c

@@ -338,7 +338,7 @@ void add_ipc_port_by_id(IDTYPE vmid, PAL_HANDLE hdl, IDTYPE type,
     if (portptr)
         *portptr = NULL;
 
-    assert(hdl && PAL_GET_TYPE(hdl));
+    assert(hdl);
     lock(&ipc_helper_lock);
 
     /* check if port with this PAL handle already exists, then we only

+ 6 - 7
Pal/regression/test_pal.py

@@ -271,13 +271,12 @@ class TC_20_SingleProcess(RegressionTestCase):
         self.assertEqual(file_exist[200:240], file_nonexist[0:40])
 
         # File Attribute Query
-        self.assertIn(
-            'Query: type = 1, size = {}'.format(len(file_exist)), stderr)
+        self.assertIn('Query: type = ', stderr)
+        self.assertIn(', size = {}'.format(len(file_exist)), stderr)
 
         # File Attribute Query by Handle
-        self.assertIn(
-            'Query by Handle: type = 1, size = {}'.format(len(file_exist)),
-            stderr)
+        self.assertIn('Query by Handle: type = ', stderr)
+        self.assertIn(', size = {}'.format(len(file_exist)), stderr)
 
         # File Mapping
         self.assertIn(
@@ -335,10 +334,10 @@ class TC_20_SingleProcess(RegressionTestCase):
             self.assertIn('Read Directory: {}'.format(p.name), stderr)
 
         # Directory Attribute Query
-        self.assertIn('Query: type = 7', stderr)
+        self.assertIn('Query: type = ', stderr)
 
         # Directory Attribute Query by Handle
-        self.assertIn('Query by Handle: type = 7', stderr)
+        self.assertIn('Query by Handle: type = ', stderr)
 
         # Directory Deletion
         self.assertFalse(pathlib.Path('dir_delete.tmp').exists())

+ 5 - 3
Pal/src/db_object.c

@@ -33,10 +33,13 @@
 int _DkObjectClose (PAL_HANDLE objectHandle)
 {
     const struct handle_ops * ops = HANDLE_OPS(objectHandle);
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
     int ret = 0;
 
     /* if the operation 'close' is defined, call the function. */
-    if (ops && ops->close)
+    if (ops->close)
         ret = ops->close(objectHandle);
 
     /*
@@ -56,7 +59,7 @@ void DkObjectClose (PAL_HANDLE objectHandle)
 {
     ENTER_PAL_CALL(DkObjectClose);
 
-    if (!objectHandle || UNKNOWN_HANDLE(objectHandle)) {
+    if (!objectHandle) {
         _DkRaiseFailure(PAL_ERROR_INVAL);
         LEAVE_PAL_CALL();
     }
@@ -64,7 +67,6 @@ void DkObjectClose (PAL_HANDLE objectHandle)
     UNTRACE_HEAP(objectHandle);
 
     int ret = _DkObjectClose(objectHandle);
-
     if (ret < 0)
         _DkRaiseFailure(-ret);
 

+ 29 - 46
Pal/src/db_streams.c

@@ -39,7 +39,6 @@ extern struct handle_ops dir_ops;
 extern struct handle_ops tcp_ops;
 extern struct handle_ops udp_ops;
 extern struct handle_ops udpsrv_ops;
-extern struct hadnle_ops udppacket_ops;
 extern struct handle_ops thread_ops;
 extern struct handle_ops proc_ops;
 extern struct handle_ops mutex_ops;
@@ -173,7 +172,7 @@ DkStreamOpen(PAL_STR uri, PAL_FLG access, PAL_FLG share, PAL_FLG create, PAL_FLG
     }
 
     assert(handle);
-    assert(PAL_GET_TYPE(handle));
+    assert(!UNKNOWN_HANDLE(handle));
 
     TRACE_HEAP(handle);
     LEAVE_PAL_CALL_RETURN(handle);
@@ -184,10 +183,10 @@ int _DkStreamWaitForClient(PAL_HANDLE handle, PAL_HANDLE* client) {
         return -PAL_ERROR_BADHANDLE;
 
     const struct handle_ops* ops = HANDLE_OPS(handle);
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
 
-    /* No ops or ops->delete being defined, inferring the handle can never
-       be deleted */
-    if (!ops || !ops->waitforclient)
+    if (!ops->waitforclient)
         return -PAL_ERROR_NOTSERVER;
 
     return ops->waitforclient(handle, client);
@@ -213,14 +212,12 @@ DkStreamWaitForClient(PAL_HANDLE handle) {
    the stream. For example, file will be deleted, socket witll be
    disconnected, etc */
 int _DkStreamDelete(PAL_HANDLE handle, int access) {
-    if (UNKNOWN_HANDLE(handle))
-        return -PAL_ERROR_BADHANDLE;
-
     const struct handle_ops* ops = HANDLE_OPS(handle);
 
-    /* No ops or ops->delete being defined, inferring the handle can never
-       be deleted */
-    if (!ops || !ops->delete)
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
+    if (!ops->delete)
         return -PAL_ERROR_NOTSUPPORT;
 
     return ops->delete(handle, access);
@@ -248,15 +245,10 @@ void DkStreamDelete(PAL_HANDLE handle, PAL_FLG access) {
    The actual behavior of stream read is defined by handler */
 int64_t _DkStreamRead(PAL_HANDLE handle, uint64_t offset, uint64_t count, void* buf, char* addr,
                       int addrlen) {
-    if (UNKNOWN_HANDLE(handle))
-        return -PAL_ERROR_BADHANDLE;
-
     const struct handle_ops* ops = HANDLE_OPS(handle);
 
-    /* if ops or ops->read is not defined, it infers that the stream can
-       never be read */
     if (!ops)
-        return -PAL_ERROR_NOTSUPPORT;
+        return -PAL_ERROR_BADHANDLE;
 
     if (!count)
         return -PAL_ERROR_ZEROSIZE;
@@ -305,13 +297,10 @@ DkStreamRead(PAL_HANDLE handle, PAL_NUM offset, PAL_NUM count, PAL_PTR buffer, P
    The actual behavior of stream write is defined by handler */
 int64_t _DkStreamWrite(PAL_HANDLE handle, uint64_t offset, uint64_t count, const void* buf,
                        const char* addr, int addrlen) {
-    if (UNKNOWN_HANDLE(handle))
-        return -PAL_ERROR_BADHANDLE;
-
     const struct handle_ops* ops = HANDLE_OPS(handle);
 
     if (!ops)
-        return -PAL_ERROR_NOTSUPPORT;
+        return -PAL_ERROR_BADHANDLE;;
 
     if (!count)
         return -PAL_ERROR_ZEROSIZE;
@@ -404,14 +393,11 @@ DkStreamAttributesQuery(PAL_STR uri, PAL_STREAM_ATTR* attr) {
 /* _DkStreamAttributesQueryByHandle for internal use. Query attribute
    of streams by their handle */
 int _DkStreamAttributesQueryByHandle(PAL_HANDLE hdl, PAL_STREAM_ATTR* attr) {
-    if (UNKNOWN_HANDLE(hdl))
-        return -PAL_ERROR_BADHANDLE;
-
     const struct handle_ops* ops = HANDLE_OPS(hdl);
 
-    assert(ops);
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
 
-    /* if ops->attrquerybyhdl is not defined, the stream cannot be queried */
     if (!ops->attrquerybyhdl)
         return -PAL_ERROR_NOTSUPPORT;
 
@@ -452,15 +438,12 @@ DkStreamAttributesSetByHandle(PAL_HANDLE handle, PAL_STREAM_ATTR* attr) {
         LEAVE_PAL_CALL_RETURN(PAL_FALSE);
     }
 
-    if (UNKNOWN_HANDLE(handle)) {
+    const struct handle_ops* ops = HANDLE_OPS(handle);
+    if (!ops) {
         _DkRaiseFailure(PAL_ERROR_BADHANDLE);
         LEAVE_PAL_CALL_RETURN(PAL_FALSE);
     }
 
-    const struct handle_ops* ops = HANDLE_OPS(handle);
-
-    assert(ops);
-
     if (!ops->attrsetbyhdl) {
         _DkRaiseFailure(PAL_ERROR_NOTSUPPORT);
         LEAVE_PAL_CALL_RETURN(PAL_FALSE);
@@ -478,9 +461,10 @@ DkStreamAttributesSetByHandle(PAL_HANDLE handle, PAL_STREAM_ATTR* attr) {
 
 int _DkStreamGetName(PAL_HANDLE handle, char* buffer, int size) {
     const struct handle_ops* ops = HANDLE_OPS(handle);
-    assert(ops);
 
-    /* if ops->getname is not defined, the stream cannot be queried */
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
     if (!ops->getname)
         return -PAL_ERROR_NOTSUPPORT;
 
@@ -504,11 +488,6 @@ PAL_NUM DkStreamGetName(PAL_HANDLE handle, PAL_PTR buffer, PAL_NUM size) {
         LEAVE_PAL_CALL_RETURN(0);
     }
 
-    if (UNKNOWN_HANDLE(handle)) {
-        _DkRaiseFailure(PAL_ERROR_BADHANDLE);
-        LEAVE_PAL_CALL_RETURN(0);
-    }
-
     int ret = _DkStreamGetName(handle, (void*)buffer, size);
 
     if (ret < 0) {
@@ -527,8 +506,10 @@ int _DkStreamMap(PAL_HANDLE handle, void** paddr, int prot, uint64_t offset, uin
 
     const struct handle_ops* ops = HANDLE_OPS(handle);
 
-    /* if ops or ops->map is not defined, the stream cannot be mapped */
-    if (!ops || !ops->map)
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
+    if (!ops->map)
         return -PAL_ERROR_NOTSUPPORT;
 
     if ((ret = ops->map(handle, &addr, prot, offset, size)) < 0)
@@ -600,12 +581,12 @@ void DkStreamUnmap(PAL_PTR addr, PAL_NUM size) {
 /* _DkStreamSetLength for internal use. This function truncate the stream
    to certain length. This call might not be support for certain streams */
 int64_t _DkStreamSetLength(PAL_HANDLE handle, uint64_t length) {
-    if (UNKNOWN_HANDLE(handle))
-        return -PAL_ERROR_BADHANDLE;
-
     const struct handle_ops* ops = HANDLE_OPS(handle);
 
-    if (!ops || !ops->setlength)
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
+    if (!ops->setlength)
         return -PAL_ERROR_NOTSUPPORT;
 
     return ops->setlength(handle, length);
@@ -645,7 +626,10 @@ int _DkStreamFlush(PAL_HANDLE handle) {
 
     const struct handle_ops* ops = HANDLE_OPS(handle);
 
-    if (!ops || !ops->flush)
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
+    if (!ops->flush)
         return -PAL_ERROR_NOTSUPPORT;
 
     return ops->flush(handle);
@@ -727,7 +711,6 @@ PAL_HANDLE DkReceiveHandle(PAL_HANDLE handle) {
     }
 
     assert(cargo);
-    assert(PAL_GET_TYPE(cargo));
     LEAVE_PAL_CALL_RETURN(cargo);
 }
 

+ 3 - 0
Pal/src/host/FreeBSD/db_object.c

@@ -99,6 +99,9 @@ static int _DkObjectWaitOne (PAL_HANDLE handle, int timeout)
 
     const struct handle_ops * ops = HANDLE_OPS(handle);
 
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
     if (!ops->wait)
         return -PAL_ERROR_NOTSUPPORT;
 

+ 4 - 1
Pal/src/host/Linux-SGX/db_object.c

@@ -91,7 +91,10 @@ static int _DkObjectWaitOne(PAL_HANDLE handle, int64_t timeout_us) {
 
     const struct handle_ops * ops = HANDLE_OPS(handle);
 
-    if (!ops || !ops->wait)
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
+    if (!ops->wait)
         return -PAL_ERROR_NOTSUPPORT;
 
     return ops->wait(handle, timeout_us);

+ 4 - 1
Pal/src/host/Linux/db_object.c

@@ -112,7 +112,10 @@ static int _DkObjectWaitOne(PAL_HANDLE handle, int64_t timeout_us) {
 
     const struct handle_ops * ops = HANDLE_OPS(handle);
 
-    if (!ops || !ops->wait)
+    if (!ops)
+        return -PAL_ERROR_BADHANDLE;
+
+    if (!ops->wait)
         return -PAL_ERROR_NOTSUPPORT;
 
     return ops->wait(handle, timeout_us);

+ 4 - 8
Pal/src/pal.h

@@ -63,11 +63,8 @@ static inline void init_handle_hdr(PAL_HDR *hdr, int pal_type) {
     hdr->flags = 0;
 }
 
-# define SET_HANDLE_TYPE(handle, t) \
-    init_handle_hdr(HANDLE_HDR(handle), pal_type_##t)
-
-# define IS_HANDLE_TYPE(handle, t)              \
-    (HANDLE_HDR(handle)->type == pal_type_##t)
+# define SET_HANDLE_TYPE(handle, t) init_handle_hdr(HANDLE_HDR(handle), pal_type_##t)
+# define IS_HANDLE_TYPE(handle, t) (HANDLE_HDR(handle)->type == pal_type_##t)
 
 #else
 typedef union pal_handle
@@ -128,7 +125,6 @@ typedef struct {
 
 /********** PAL TYPE DEFINITIONS **********/
 enum {
-    pal_type_none = 0,
     pal_type_file,
     pal_type_pipe,
     pal_type_pipesrv,
@@ -150,10 +146,10 @@ enum {
 };
 
 
-/* PAL identifier poison value */
-#define PAL_IDX_POISON          ((PAL_IDX) -1)
+#define PAL_IDX_POISON          ((PAL_IDX)-1) /* PAL identifier poison value */
 #define PAL_GET_TYPE(h)         (HANDLE_HDR(h)->type)
 #define PAL_CHECK_TYPE(h, t)    (PAL_GET_TYPE(h) == pal_type_##t)
+#define UNKNOWN_HANDLE(handle)  (PAL_GET_TYPE(handle) >= PAL_HANDLE_TYPE_BOUND)
 
 typedef struct { PAL_PTR start, end; }  PAL_PTR_RANGE;
 

+ 1 - 4
Pal/src/pal_internal.h

@@ -127,7 +127,7 @@ extern const struct handle_ops * pal_handle_ops [];
 static inline const struct handle_ops * HANDLE_OPS (PAL_HANDLE handle)
 {
     int _type = PAL_GET_TYPE(handle);
-    if (_type <= 0 || _type >= PAL_HANDLE_TYPE_BOUND)
+    if (_type >= PAL_HANDLE_TYPE_BOUND)
         return NULL;
     return pal_handle_ops[_type];
 }
@@ -163,9 +163,6 @@ static inline uint64_t hash64 (uint64_t key)
 extern PAL_HANDLE _h;
 #define HANDLE_SIZE(type)  (sizeof(*_h))
 
-#define UNKNOWN_HANDLE(handle)     \
-    (PAL_GET_TYPE(handle) == 0 || PAL_GET_TYPE(handle) >= PAL_HANDLE_TYPE_BOUND)
-
 static inline int handle_size (PAL_HANDLE handle)
 {
     return sizeof(*handle);