Skip to content

aosp: report POSIX symlink st_size from lstat - #11332

Open
rogerzanoni wants to merge 1 commit into
youtube:mainfrom
rogerzanoni:aosp-lstat
Open

aosp: report POSIX symlink st_size from lstat#11332
rogerzanoni wants to merge 1 commit into
youtube:mainfrom
rogerzanoni:aosp-lstat

Conversation

@rogerzanoni

Copy link
Copy Markdown
Collaborator

POSIX requires a symlink's st_size to be the length of its target path. Android's /data filesystem reports a different value.

Fixes the following issues in tests
posix_lstat_test.cc:97 EXPECT_EQ(sb.st_size, target_filename.length())

Bug: 532068409

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Gemini Suggested Commit Message


starboard: Fix symlink st_size in fstatat

POSIX requires a symbolic link's st_size to be the length of its target
path. However, Android's /data filesystem may report a different value,
leading to inconsistencies and test failures.

The Starboard POSIX wrapper for fstatat now ensures the reported size
matches the target path length when querying symbolic links without
following them.

Bug: 532068409

💡 Pro Tips for a Better Commit Message:

  1. Influence the Result: Want to change the output? You can write custom prompts or instructions directly in the Pull Request description. The model uses that text to generate the message.
  2. Re-run the Generator: Post a comment with: /generate-commit-message

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request updates __abi_wrap_fstatat to normalize symlink sizes on platforms like Android by querying the target path length via readlinkat. The reviewer pointed out that readlinkat could fail and pollute errno even when the wrapper returns success, and suggested saving and restoring errno around the call to prevent this side effect.

Comment thread starboard/shared/modular/starboard_layer_posix_stat_abi_wrappers.cc Outdated
@sacuff
sacuff enabled auto-merge (squash) July 21, 2026 20:18

@andrewsavage1 andrewsavage1 left a comment

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.

We should only change the posix wrappers to have it conform with posix specifications, not use it to support platform-specific quirks. Unless it's not possible, this change should be implemented as an android-specific wrapper, similar to the asset management with stat/open

POSIX requires a symlink's st_size to be the length of its target path. Android's
/data filesystem reports a different value.

Fixes the following issues in tests
  posix_lstat_test.cc:97 EXPECT_EQ(sb.st_size, target_filename.length())

Bug: 532068409
auto-merge was automatically disabled July 27, 2026 01:37

Head branch was pushed to by a user without write access

@rogerzanoni

Copy link
Copy Markdown
Collaborator Author

@andrewsavage1 moved the fix to the posix_emu wrapper

@andrewsavage1

Copy link
Copy Markdown
Contributor

@rogerzanoni you need to fix the lint checks here

@andrewsavage1
andrewsavage1 enabled auto-merge (squash) July 27, 2026 13:32
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.

3 participants