You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2022/02/22 20:58:51 UTC
[GitHub] [celix] pnoltes commented on a change in pull request #400: Improved error codes for remote services
pnoltes commented on a change in pull request #400:
URL: https://github.com/apache/celix/pull/400#discussion_r812344354
##########
File path: libs/utils/include/celix_errno.h
##########
@@ -99,6 +150,10 @@ const char* celix_strerror(celix_status_t status);
#define CELIX_FRAMEWORK_EXCEPTION (CELIX_START_ERROR + 7)
Review comment:
I think the error could be replaced with something like:
```
const celix_status_t CELIX_FRAMEWORK_EXCEPTION = CELIX_ERROR_MAKE(CELIX_FACILITY_SYSTEM, 7);
const celix_status_t CELIX_FILE_IO_EXCEPTION = CELIX_ERROR_MAKE(CELIX_FACILITY_SYSTEM, 9);
//etc
```
That we make use of the new error code approach and also create an example.
##########
File path: libs/utils/include/celix_errno.h
##########
@@ -61,6 +73,44 @@ typedef int celix_status_t;
*/
const char* celix_strerror(celix_status_t status);
+/*!
+ * Customer error code mask
+ *
+ */
+#define CELIX_CUSTOMER_ERR_MASK 0x20000000
+
+/*!
+ * The facility of system error code,
+ * \note Error code 0 indicates success,it is not system error code.
+ */
+#define CELIX_FACILITY_SYSTEM 0
Review comment:
De we want a facility `0` code or always ensure a not `0` to more easily see of its a valid error code.
##########
File path: libs/utils/include/celix_errno.h
##########
@@ -61,6 +73,44 @@ typedef int celix_status_t;
*/
const char* celix_strerror(celix_status_t status);
+/*!
+ * Customer error code mask
+ *
+ */
+#define CELIX_CUSTOMER_ERR_MASK 0x20000000
+
+/*!
+ * The facility of system error code,
+ * \note Error code 0 indicates success,it is not system error code.
+ */
+#define CELIX_FACILITY_SYSTEM 0
+
+/*!
+ * The facility of celix default error code
+ *
+ */
+#define CELIX_FACILITY_NULL 1
Review comment:
I am not sure why a `CELIX_FACILITY_NULL` is needed, is the `CELIX_FACILITY_SYSTEM` not a more logical default facility error value?
##########
File path: libs/utils/include/celix_errno.h
##########
@@ -99,6 +150,10 @@ const char* celix_strerror(celix_status_t status);
#define CELIX_FRAMEWORK_EXCEPTION (CELIX_START_ERROR + 7)
#define CELIX_FILE_IO_EXCEPTION (CELIX_START_ERROR + 8)
#define CELIX_SERVICE_EXCEPTION (CELIX_START_ERROR + 9)
+/*!
+ * Exception indicating a problem with a interceptor
+ */
+#define CELIX_INTERCEPTOR_EXCEPTION (CELIX_START_ERROR + 10)
#define CELIX_ENOMEM ENOMEM
Review comment:
Maybe also give CELIX_ENOMEM an own value and directly couple it to the C ENOMEM
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@celix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org