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.

Sign up for GitHub

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

fix: fix issue#8 #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: fix issue#8 #11

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e24fb2
fix: fix issue#4
Yangfisher1 Nov 15, 2021
a2253c3
fix:fix issue#8
Yangfisher1 Nov 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions rdcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1090,6 +1090,7 @@ static int rdcp_test_client(struct rdcp_cb *cb)
struct ibv_send_wr *bad_wr;
int size;
long total_size = 0;
struct stat filebuf;

if (!cb->use_null) {
cb->fd = open(cb->metadata.src_path, O_RDONLY);
Expand All @@ -1098,14 +1099,17 @@ static int rdcp_test_client(struct rdcp_cb *cb)
perror("Couldn't open file");
goto out;
}
cb->fp = fdopen(cb->fd, "rb");
fseek(cb->fp, 0, SEEK_END);
cb->metadata.size = ftell(cb->fp);
// cb->fp = fdopen(cb->fd, "rb");
// fseek(cb->fp, 0, SEEK_END);
// cb->metadata.size = ftell(cb->fp);
Copy link
Owner

Choose a reason for hiding this comment

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

dont leave commented out code.

stat(cb->metadata.src_path, &filebuf);
cb->metadata.size = filebuf.st_size;
Copy link
Owner

Choose a reason for hiding this comment

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

how is using stat() instead of fseek() helps here? stat() is great and has better performance but beside that i dont see how it fix an issue. if you can change your commit to explain to use stat() instead of fseek() its fine by me to accept the commit anyway. but it is requiring some explanation why/how it fixes the issue with small files.

Copy link
Author

Choose a reason for hiding this comment

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

I'm inspired by issue#8. After read the issue, I searched the Internet for the difference between stat() and fseek(). I believed that it might have some conflicts between a file pointer(cb->fp) and a file descriptor(cb->fd), so it'd better use just a file pointer or just a file descriptor rather than hybird, and using stat() can simply and easily get the whole file's length without moving position(which I thought the bug happened), but actually I didn't know exactly why this happened, sorry.

Copy link
Owner

Choose a reason for hiding this comment

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

ok and it is fine by me but need to refactor the patch to have a meaningful title and description.
you can just say use stat() to get the file size and the body will be to fix issue #8 and its fine by me. also need to clean commented out code. there is no point to leave this.

Copy link
Owner

Choose a reason for hiding this comment

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

ok and it is fine by me but need to refactor the patch to have a meaningful title and description.
you can just say use stat() to get the file size and the body will be to fix issue #8 and its fine by me. also need to clean commented out code. there is no point to leave this.


if (cb->metadata.size <= 0) {
perror("size error");
goto out;
}
fseek(cb->fp, 0, SEEK_SET);
// fseek(cb->fp, 0, SEEK_SET);
} else {
cb->metadata.size = 0;
}
Expand Down