Skip to content

fix #263 #265

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

Merged
merged 5 commits into from
Jan 4, 2019
Merged

fix #263 #265

merged 5 commits into from
Jan 4, 2019

Conversation

yatli
Copy link

@yatli yatli commented Jan 3, 2019

No description provided.

@yatli yatli requested review from leoxia and shaobin January 3, 2019 12:55
@yatli yatli requested a review from LiangHe-MS January 3, 2019 12:58
@@ -96,7 +96,7 @@ namespace Trinity
}
bool Read(char* buffer, int32_t start, int32_t count)
{
return (count == fread(buffer + start, count, 1, fp));
return (1 == fread(buffer + start, count, 1, fp));
Copy link
Contributor

Choose a reason for hiding this comment

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

The parameter name 'count' is confusing. It may be better to rename it to 'element_size' or just 'size' to be consistent with fread()'s parameter name. For the reference, here is the prototype of fread: size_t fread(void *ptr, size_t size, size_t nmemb, FILE *stream).

Copy link
Author

Choose a reason for hiding this comment

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

sure, we'd better say size instead of count -- fool me once

Copy link
Author

Choose a reason for hiding this comment

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

done.

@yatli yatli merged commit 5413714 into master Jan 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants