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.

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

lib/vfscore: Fix getcwd to use the kernel API #1268

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

andreittr
Copy link
Contributor

Description of changes

Previously the getcwd syscall behaved according to the libc API instead of the kernel one, returning a pointer to the buffer containing the path, as well as potentially allocating memory.
This change fixes this, providing the correct API: userspace is solely responsible for memory allocation, and the syscall returns the length of the output path.

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): N/A
  • Platform(s): N/A
  • Application(s): N/A

Additional configuration

Build with vfscore enabled (e.g., app-elfloader). Bincompat golang will, for some PWDs, return error from os.Getwd without this fix.

Previously the getcwd syscall behaved according to the libc API instead
of the kernel one, returning a pointer to the buffer containing the
path, as well as potentially allocating memory.
This change fixes this, providing the correct API: userspace is solely
responsible for memory allocation, and the syscall returns the length of
the output path.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
@andreittr andreittr requested a review from a team as a code owner January 17, 2024 20:09
@github-actions github-actions bot added area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/vfscore VFS Core Interface labels Jan 17, 2024
@razvand razvand requested review from StefanJum and removed request for a team January 18, 2024 08:38
@razvand razvand added this to the v0.16.1 (Telesto) milestone Jan 18, 2024
Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

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

Works fine, thanks.
Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

Copy link
Member

@skuenzer skuenzer left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Simon Kuenzer simon@unikraft.io

Copy link
Member

@skuenzer skuenzer left a comment

Choose a reason for hiding this comment

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

Great, I also checked that it works and code update looks fine. Thanks!

Approved-by: Simon Kuenzer simon@unikraft.io

@razvand razvand changed the base branch from staging to staging-1268 January 18, 2024 13:37
@razvand razvand merged commit 0fc198d into unikraft:staging-1268 Jan 18, 2024
11 of 12 checks passed
razvand pushed a commit that referenced this pull request Jan 18, 2024
Previously the getcwd syscall behaved according to the libc API instead
of the kernel one, returning a pointer to the buffer containing the
path, as well as potentially allocating memory.
This change fixes this, providing the correct API: userspace is solely
responsible for memory allocation, and the syscall returns the length of
the output path.

Signed-off-by: Andrei Tatar <andrei@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Reviewed-by: Simon Kuenzer <simon@unikraft.io>
Approved-by: Simon Kuenzer <simon@unikraft.io>
GitHub-Closes: #1268
@andreittr andreittr deleted the ttr/getcwd-kapi branch January 22, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ lib/vfscore VFS Core Interface
Projects
Development

Successfully merging this pull request may close these issues.

4 participants