mike-jumper commented on code in PR #457: URL: https://github.com/apache/guacamole-server/pull/457#discussion_r1310648836
########## src/libguac/guacamole/rwlock.h: ########## @@ -42,15 +42,15 @@ * An error code indicating that the calling thread is attempting to release a * lock that it does not control. */ -#define GUAC_REEANTRANT_LOCK_ERROR_DOUBLE_RELEASE 1 +#define GUAC_RWLOCK_ERROR_DOUBLE_RELEASE 1 /** * The lock cannot be acquired because the lock has been already been * reentrantly acquired too many times, exhausting the capacity of this library - * to track this lock. The lock must be released using guac_release_lock() + * to track this lock. The lock must be released using guac_rwlock_release_lock() * before it can be reacquired. */ -#define GUAC_REEANTRANT_LOCK_ERROR_TOO_MANY 2 +#define GUAC_RWLOCK_ERROR_TOO_MANY 2 Review Comment: Rather than define new error codes and constants, I think it would be best to: 1. Use a non-zero return generically to represent that the operation failed. 2. Use `guac_error` (the libguac analogy to `errno`), `guac_error_message`, and the established `guac_status` constants (like `GUAC_STATUS_INVALID_ARGUMENT` and `GUAC_STATUS_TOO_MANY`) to make error details available. ########## src/libguac/guacamole/client.h: ########## @@ -30,7 +30,7 @@ #include "client-types.h" #include "client-constants.h" #include "layer-types.h" -#include "reentrant-rwlock.h" +#include "guacamole/rwlock.h" Review Comment: This should just be: ```c #include "rwlock.h" ``` within files that are already within the `guacamole/` include path. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
