Opened 3 weeks ago
Last modified 3 weeks ago
#854 new defect
Avoid using lseek() to set file position
| Reported by: | Olly Betts | Owned by: | Olly Betts |
|---|---|---|---|
| Priority: | normal | Milestone: | 2.1.1 |
| Component: | Backend-Glass | Version: | 2.1.0 |
| Severity: | normal | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Operating System: | All |
Description
Constructing a Database object for a glass database from a file descriptor currently uses lseek() to set the file position before reading the version file.
That's not such a problem when we're opening the database as the current file position is used as the offset in the file to read the database from, but a call to Database::reopen() will also do this.
We should ideally only make one call to lseek() to read (and not change) the file position when the database is opened, and then use pread() for all reads of the version file (like we already do for the database tables).
This would make it safer to use dup() to create an fd to pass to Xapian (since such an fd shares its file position with the fd it was created from).
Background: https://github.com/openzim/libzim/pull/1119

I have a working patch for this which I plan to merge for the next 2.x release.
I tried to write a note for the docs about it, and it seems awkward to say that you can use
dup()here safely provided the platform providespread()and implements it with the correct POSIX semantics because as the author of code doing this you don't really know what platforms it may get run on, and they can't easilyassert(current_platform_has_working_pread();.I suspect there aren't really any Unix-like platforms without a properly working
pread()nowadays, but it's hard to really know.We could perhaps state it as a requirement for Xapian, but then we should really test for it. A configure test to check it wouldn't work when cross-compiling. We could test it in the testsuite, but not everyone runs that.
More broadly, we have quite a lot of code written to provide compatibility when some feature is missing or not working correctly, but some is 20+ years old at this point. Relevant here for example is that we have a workaround added in 2004 to block trying to use broken
pread()/pwrite()on HP-UX (they didn't work when Large File Support was enabled); it's quite likely they aren't broken in more recent releases, though it's probably moot as Wikipedia says HP-UX was discontinued last year.Another example is handling for
O_CLOEXECand related flags and functions not being available or being available but not working - when these were being added you needed to also be running a kernel with support, but at least for Linux everyone will be now and I'd expect all Unix-like platforms would have added support some time ago.