detect logical sector size #1875
Conversation
lightmark
force-pushed
the
dynamic_local_sector_size
branch
from
77a17ea
to
61f092f
Compare
|
@lightmark updated the pull request - view changes |
|
@lightmark updated the pull request - view changes |
lightmark
force-pushed
the
dynamic_local_sector_size
branch
from
61f092f
to
52bcf83
Compare
|
@lightmark updated the pull request - view changes |
|
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
util/io_posix.cc
Outdated
| return size; | ||
| } | ||
| } | ||
| #endif // namespace |
There was a problem hiding this comment.
this line doesn't end the namespace :p
| @@ -684,40 +684,39 @@ class IoctlFriendlyTmpdir { | |||
| std::string dir_; | |||
| }; | |||
|
|
|||
| TEST_P(EnvPosixTestWithParam, PositionedAppend) { | |||
| if (direct_io_ && env_ == Env::Default()) { | |||
util/io_posix.cc
Outdated
| std::string fname = device_dir + "/queue/logical_block_size"; | ||
| FILE* fp; | ||
| char* line = nullptr; | ||
| size_t len; |
There was a problem hiding this comment.
from the man-page:
If *lineptr is set to NULL and *n is set 0 before the call, then getline() will allocate a buffer for storing the line. This buffer should be freed by the user program even if getline() failed.
I think you need to (1) initialize len to zero, (2) free "line" if it's non-nullptr on line 98.
util/io_posix.cc
Outdated
| size_t len; | ||
| fp = fopen(fname.c_str(), "r"); | ||
| if (fp == nullptr || getline(&line, &len, fp) == -1) { | ||
| return kDefaultPageSize; |
There was a problem hiding this comment.
you also may need to close fp here
|
@lightmark updated the pull request - view changes - changes since last import |
|
@lightmark updated the pull request - view changes - changes since last import |
|
@lightmark updated the pull request - view changes - changes since last import |
util/io_posix.cc
Outdated
| use_direct_io_(options.use_direct_reads) { | ||
| use_direct_io_(options.use_direct_reads), | ||
| logical_sector_size_(kDefaultPageSize) { | ||
| #ifdef OS_LINUX |
There was a problem hiding this comment.
consider returning kDefaultPageSize from GetLogicalBufferSize() if OS_LINUX not defined, then you don't need to set it twice everywhere
|
@lightmark updated the pull request - view changes - changes since last import |
|
@lightmark has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
querying logical sector size from the device instead of hardcoding it for linux platform.