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

Read and write syscalls may read less bytes than requested #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lucckb
Copy link

@lucckb lucckb commented Dec 14, 2020

read/write syscalls may read/write less bytes then
required in the size argument, so when the larger block size
is passed to the library lfsfuse fails because read/write returns
less bytes than requested.

read/write syscalls may read/write less bytes then
requied in the size, so on the larger block size
lfs fuse library fails because read/write returns
less bytes than required.
Copy link
Member

@geky geky left a comment

Choose a reason for hiding this comment

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

Hi @lucckb, thanks for creating a PR!

The main thing is that this patch needs to match the style of the existing code. Squiggly brackets on same line of if statements, no spaces around conditions, spaces after if/while, space between pointer and type, no do-while loops (use while (true) with a break instead), and the ternary operators formatted as (res == 0) ? LFS_ERR_IO : -errno for consistency.

wrb += res;
size -= res;
}
} while(0);
Copy link
Member

Choose a reason for hiding this comment

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

I may be missing something, but why does this do/while loop have 0 as a condition?

@geky
Copy link
Member

geky commented Aug 22, 2023

I've created an alternative PR that should fix this issue based on this PR: #54

Sorry about not bringing this one in and the late resolution. Thanks for the original 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.

2 participants