On Wed, Jun 28, 2023 at 09:41:13PM +0800, Zhangjin Wu wrote:
static __attribute__((unused)) void *mmap(void *addr, size_t length, int prot, int flags, int fd, off_t offset) {
- void *ret = sys_mmap(addr, length, prot, flags, fd, offset);
- if ((unsigned long)ret >= -4095UL) {
SET_ERRNO(-(long)ret);ret = MAP_FAILED;- }
- return ret;
- return (void *)__sysret((unsigned long)sys_mmap(addr, length, prot, flags, fd, offset));
}
One point regarding this one. By doing so, we're hard-coding the fact that we consider that MAP_FAILED is always -1. I'm not necessarily against it, but this implication can be confusing for those searching where it's being set. I would suggest putting a comment before the mmap() function saying:
/* Note that on Linux MAP_FAILED is -1 so we can use the generic __sysret()
- which returns -1 upon error and still satisfy user land that checks for
- MAP_FAILED.
*/
Since it's an assumed choice that theoretically could affect portability, it should be reflected in the commit message as well (and we all know it does not have any impact).
Yeah, we do need such a comment and commit message note, thanks.
Best regards, Zhangjin
Thanks! Willy