Integer overflow bug in fastfilereader and offset parameter to stream a file #533
Conversation
|
I also found that there was an integer overflow bug in fastfilereader module. The NUM2INT and INT2NUM were at fault. Changed them to NUM2LONG and LONG2NUM respectively. |
|
Looks reasonable - please rebase the PR, and squash to 2 commits: one for the int/long fixes, the other for the new offset feature. |
ext/cmain.cpp
Outdated
| @@ -785,6 +785,8 @@ extern "C" int evma_send_file_data_to_connection (const unsigned long binding, c | |||
| return -1; | |||
| } | |||
|
|
|||
| // seek to the offset first | |||
| lseek(Fd, offset, SEEK_SET); | |||
|
Hi - I noticed the added commits, but I don't think it came out as intended. You might have rebased locally, then done a 'git pull' and merged origin/master back into your local master before you pushed? The trick with a rebase is that you must force-push once you have the commits on your local branch organized as you wish, so that the origin matches local. |
|
I should have commented that I messed this whole rebase business. I'll try and clean this up again. If I am not able to clean it, I'll submit a fresh pull request. I hope that should be fine. |
Fixed bug in reading file greater than 2GB in size converted INT to LONG to support >2GB file size converted to NUM to LONG sorry, my bad. used wrong macro yet another overflow when adding start and offset updated the function definition converted to long to unsigned long
|
Hi Aaron, after some hiccups and doing multiple reverts, I finally managed to clean the commit tree (and learned some new git tricks). Do let me know if this looks good. Thanks for your patience. |
|
Yes, this looks much better! From reading this again, I agree with the NUM2ULONG changes. Actually, I wonder if those should be NUM2SIZET http://rxr.whitequark.org/mri/source/include/ruby/ruby.h#672 for 64-bit files. I am not comfortable with the offset parameter. The function The best solution would be support for |
Nothing too fancy. Just had a requirement to stream a file from a certain offset (in case the file was not streamed completely last time). So went ahead and added this feature in the eventmachine.
Please accept this pull request if you feel it is a good feature to have.