Cynerd commented on code in PR #17441: URL: https://github.com/apache/nuttx/pull/17441#discussion_r2598418908
########## drivers/can/can.c: ########## @@ -518,7 +518,7 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, /* ret can be more than buflen due to roundup, so return at most buflen */ - return ret ? MIN(ret, buflen) : -EMSGSIZE; + return ret ? MIN(ret, (int)buflen) : -EMSGSIZE; Review Comment: The comparison of unsigned and signed values is definitely incorrect. I would just not type change it to int, but instead to `ssize_t` at least. The same issue is in the `can_write` function, as I just used the same logic there, so I would include that in this commit as well. Or if we want to be fully correct (as changing `size_t` to `ssize_t` can crop), we can use what @xiaoxiang781216 suggested, but an additional condition has to be added to not return `0` as message size error should be reported (instead of invalid EOF). Edit: Yeah, we return `ssize_t`, so the function can't handle anything more than that (not a full `size_t`, which is not required anyway), so the first solution is probably the best. Just use `ssize_t` instead of `int` in my opinion. Edit2: Actually, the `res` should have been `ssize_t` to match the type we are returning, not `int`. So I would go with that. -- 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]
