Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Libuv 1.44.0 uv_os_get_passwd signed -> unsigned change breaks our binding #589

Closed
zhaozg opened this issue Mar 11, 2022 · 1 comment · Fixed by #590
Closed

Libuv 1.44.0 uv_os_get_passwd signed -> unsigned change breaks our binding #589

zhaozg opened this issue Mar 11, 2022 · 1 comment · Fixed by #590

Comments

@zhaozg
Copy link
Member

zhaozg commented Mar 11, 2022

After libuv version v1.44.0, we got two alerts. see.
@squeek502
Copy link
Member

squeek502 commented Mar 11, 2022 •
edited
Loading

Relevant Libuv change: libuv/libuv#3476

Might need to change those comparisons for 1.44.0 and above only, so that we keep compatibility when building against earlier Libuv versions.

Made a comment on the Libuv PR: libuv/libuv#3476 (comment)

@squeek502 squeek502 changed the title LGTM alerts Libuv 1.44.0 uv_os_get_passwd signed -> unsigned change breaks our binding Mar 12, 2022
squeek502 added a commit to squeek502/luv that referenced this issue Mar 12, 2022
@squeek502
os_get_passwd: Explicitly compare uid/gid against -1 instead of >= 0
8485c8e 
In Libuv 1.44.0, uv_passwd_t.uid/gid was changed to be unsigned, so the >= 0 check would always return true. Changing to compare against -1 explicitly fixes luvit#589 while still maintaining correct functionality when built against previous Libuv versions. Note that with Libuv 1.44.0 and above, this relies on implicitly casting -1 to unsigned long (the type of `uv_passwd_t.uid`/`uv_passwd_t.gid`).
@squeek502 squeek502 mentioned this issue Mar 12, 2022
Merged
squeek502 added a commit that referenced this issue Mar 12, 2022
@squeek502
os_get_passwd: Explicitly compare uid/gid against -1 instead of >= 0 (#…
4e7e56c 
…590)

In Libuv 1.44.0, uv_passwd_t.uid/gid was changed to be unsigned, so the >= 0 check would always return true. Changing to compare against -1 explicitly fixes #589 while still maintaining correct functionality when built against previous Libuv versions. Note that with Libuv 1.44.0 and above, this relies on implicitly casting -1 to unsigned long (the type of `uv_passwd_t.uid`/`uv_passwd_t.gid`).
squeek502 added a commit to squeek502/luv that referenced this issue Mar 12, 2022
@squeek502
os_get_passwd: Fix potential omission of gid/uid values in non-Window…
4b35ef6 
…s environments

-1 only has special meaning on Windows, and gid and uid are always set to -1 on Windows, so we can just omit the gid and uid fields on Windows instead of checking for -1. This is a better fix for luvit#589, and a follow up to luvit#590.
@squeek502 squeek502 mentioned this issue Mar 12, 2022
Merged
zhaozg pushed a commit that referenced this issue Mar 12, 2022
@squeek502 @zhaozg
os_get_passwd: Fix potential omission of gid/uid values in non-Window…
938b405 
…s environments

-1 only has special meaning on Windows, and gid and uid are always set to -1 on Windows, so we can just omit the gid and uid fields on Windows instead of checking for -1. This is a better fix for #589, and a follow up to #590.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

os_get_passwd: Explicitly compare uid/gid against -1 instead of >= 0 squeek502/luv
2 participants
@zhaozg @squeek502