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

Truncate inode number when writing to st_ino_truncated. NFC #19568

Closed
wants to merge 1 commit into from

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jun 8, 2023

This field is designed to hold a potentially truncated inode number, as opposes the st_ino which hold the full 64-bit value.

Fixes: #19567

This field is designed to hold a potentially truncated inode number,
as opposes the st_ino which hold the full 64-bit value.

Fixes: #19567
@sbc100 sbc100 requested review from kripken and walkingeyerobot June 8, 2023 22:07
@sbc100
Copy link
Collaborator Author

sbc100 commented Jun 8, 2023

I know you are never supposed to say such things.. but I'm not sure its worth the effort to add a test for this

Copy link
Collaborator

@walkingeyerobot walkingeyerobot left a comment

Choose a reason for hiding this comment

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

this fixes my issue! thanks!

@@ -52,7 +52,7 @@ var SyscallsLibrary = {
throw e;
}
{{{ makeSetValue('buf', C_STRUCTS.stat.st_dev, 'stat.dev', 'i32') }}};
{{{ makeSetValue('buf', C_STRUCTS.stat.__st_ino_truncated, 'stat.ino', 'i32') }}};
{{{ makeSetValue('buf', C_STRUCTS.stat.__st_ino_truncated, 'stat.ino >>> 0', 'i32') }}};
Copy link
Member

Choose a reason for hiding this comment

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

What effect does this have? HEAP32[x] = y will automatically do the effect of |0 to y as it is written, won't it? Is there some subtle signed/unsigned issue I am missing?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, is it hitting the recent assertion you added @sbc100 that checks the value fits?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup.. nice to see them doing their job!

@sbc100
Copy link
Collaborator Author

sbc100 commented Jun 8, 2023

Closing in favor of #19569

@sbc100 sbc100 closed this Jun 8, 2023
@sbc100 sbc100 deleted the fix_st_ino_truncated branch July 13, 2024 00:06
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