jmuehlner commented on code in PR #525:
URL: https://github.com/apache/guacamole-server/pull/525#discussion_r1777850195
##########
src/libguac/client.c:
##########
@@ -132,7 +132,8 @@ guac_stream* guac_client_alloc_stream(guac_client* client) {
return NULL;
/* Allocate stream */
- stream_index = guac_pool_next_int(client->__stream_pool);
+ stream_index = guac_pool_next_int_below_or_die(client->__stream_pool,
Review Comment:
Right above this we do a non-threadsafe check against
`GUAC_CLIENT_MAX_STREAMS`, which will cause the function to return `NULL` - do
we still need that? Does the new check not correctly handle this?
And if the documented behavior of this function is to return `NULL` when out
of streams, why are we dying here instead?
It seems like we'd want to maintain that `NULL` return for backwards
compatibility, but also I'd point out that none of the callers of these
`alloc_stream()` functions look to actually check if the return value is
non-NULL before using it...
--
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-unsubscr...@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
