Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

WIP: Fix libpsl-native to build and pass tests on FreeBSD#7208

Closed
mateusrodrigues wants to merge 4 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
mateusrodrigues:freebsd-libpslmateusrodrigues/PowerShell:freebsd-libpslCopy head branch name to clipboard
Closed

WIP: Fix libpsl-native to build and pass tests on FreeBSD#7208
mateusrodrigues wants to merge 4 commits into
PowerShell:masterPowerShell/PowerShell:masterfrom
mateusrodrigues:freebsd-libpslmateusrodrigues/PowerShell:freebsd-libpslCopy head branch name to clipboard

Conversation

@mateusrodrigues

Copy link
Copy Markdown

PR Summary

As cited in #7207, these are the changes I've made to libpsl-native to fix all tests on FreeBSD. The only two files that needed changes were src/getuserfrompid.cpp and src/isfile.cpp:

  • In isfile.cpp, I've changed the logic to verify whether something is a file or not by using a macro available in sys/stat.h. This changed allowed IsFileTest.RootIsFile to pass.
  • In getuserfrompid.cpp, a FreeBSD-specific logic wasn't defined, so NULL was being returned every time. Therefore, I've added a logic similar to the macOS one with a subtle difference in the return statement. This allowed GetUserFromPid.Success to pass.

PR Checklist

@msftclas

msftclas commented Jun 29, 2018

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

}

// TODO: Real of effective user ID?
return GetPwUid(oldp.ki_uid);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ki_uid returns effective user id, where as ki_ruid returns real user id and ki_svuid returns saved effective user id. Which one should we look for in this case?

if (lstat(path, &buf) == -1)
{
// TODO: throw error on path doesn't exist?
return false;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If path does not exist, is returning false enough?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unit test test-isfile.cpp currently expects false for a file that does not exit. See https://github.com/PowerShell/PowerShell/blob/master/src/libpsl-native/test/test-isfile.cpp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I would suggest testing for path before using it; something like the following:

if (path == NULL || lstat(path, &buf) == -1)

@daxian-dbw daxian-dbw self-assigned this Jun 29, 2018
@iSazonov

Copy link
Copy Markdown
Collaborator

Did we migrate the psl to https://github.com/PowerShell/PowerShell-Native?

@iSazonov

Copy link
Copy Markdown
Collaborator

After merging #7129 we will no longer use src/isfile.cpp.

@TravisEz13

Copy link
Copy Markdown
Member

@adityapatwardhan Can we answer @iSazonov 's question? When should we stop taking PRs here?

@adityapatwardhan

Copy link
Copy Markdown
Member

@mateusrodrigues Could you submit this PR to https://github.com/PowerShell/PowerShell-Native. We are moving the native code to a separate repo. Thanks for your contribution.

@mateusrodrigues

Copy link
Copy Markdown
Author

@adityapatwardhan just moved the PR over

@adityapatwardhan

Copy link
Copy Markdown
Member

@mateusrodrigues Thanks! I will close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.