fix: fix issue#8 #11
fix: fix issue#8 #11
Conversation
|
thanks for submitting fixes. can you fix the commit msgs to have a title and description if needed to explain the fix. the fix issue line can be as part of the description. |
| @@ -1179,7 +1179,7 @@ static int rdcp_test_client(struct rdcp_cb *cb) | |||
| VERBOSE_LOG(1, "done\n"); | |||
|
|
|||
| out: | |||
| fclose(cb->fp); | |||
| // fclose(cb->fp); | |||
There was a problem hiding this comment.
dont leave commented out code. also how does it fix anything? why do skip closing the file pointer?
| cb->metadata.size = ftell(cb->fp); | ||
| // cb->fp = fdopen(cb->fd, "rb"); | ||
| // fseek(cb->fp, 0, SEEK_END); | ||
| // cb->metadata.size = ftell(cb->fp); |
| // fseek(cb->fp, 0, SEEK_END); | ||
| // cb->metadata.size = ftell(cb->fp); | ||
| stat(cb->metadata.src_path, &filebuf); | ||
| cb->metadata.size = filebuf.st_size; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Fix the issue#8, now small files can also be transferred successfully.