yamt commented on code in PR #10869: URL: https://github.com/apache/nuttx/pull/10869#discussion_r1438138798
########## include/sys/types.h: ########## @@ -121,20 +121,27 @@ typedef uint16_t size_t; typedef int16_t ssize_t; typedef uint16_t rsize_t; +/* uid_t is used for user IDs + * gid_t is used for group IDs. + */ + +typedef int16_t uid_t; +typedef int16_t gid_t; + #else /* CONFIG_SMALL_MEMORY */ typedef _size_t size_t; typedef _ssize_t ssize_t; typedef _size_t rsize_t; -#endif /* CONFIG_SMALL_MEMORY */ - /* uid_t is used for user IDs * gid_t is used for group IDs. */ -typedef int16_t uid_t; -typedef int16_t gid_t; +typedef unsigned int uid_t; +typedef unsigned int gid_t; Review Comment: > @anchao I think that the main question raised by @yamt is related to `int16_t` (signed type) vs `unsigned int` (unsigned type). The comparison and other operations like bit shifting can behave differently depending on that, so the concern is reasonable. thank you for explanation. it's exactly what i concern. > On the other hand we already have similar precedent for `time_t`. https://github.com/apache/nuttx/pull/11466 > Usage of `CONFIG_SMALL_MEMORY` for uid/gid vs separate configuration option is a separate question to discuss. @yamt what are the concerns on utilising `CONFIG_SMALL_MEMORY` option for uid/gid if both cases would be signed (or unsigned)? CONFIG_SMALL_MEMORY is meant for memory management, right? i'd suggent to have a dedicated config, say, CONFIG_32BIT_UID, and make it default to !CONFIG_DEFAULT_SMALL. -- 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]
