file sync: windows: add checks for unix/win behaviors #112
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Eduardo Silva <[email protected]>
Signed-off-by: Eduardo Silva <[email protected]>
This commit fixes three critical inconsistencies in Windows file handling that could cause resource leaks, crashes, or data corruption: 1. cio_file_native_delete: Add safety check before deletion - Now checks if file is open or mapped before attempting deletion - Returns CIO_ERROR if file is in use (consistent with Unix behavior) - Prevents resource leaks and potential crashes 2. cio_file_native_sync: Add map validation before access - Now checks if file is mapped before accessing cf->map - Returns CIO_ERROR if file is not mapped - Prevents access violations when cf->map is NULL 3. cio_file_native_map: Fix CreateFileMappingA size parameter - Changed from (0, 0) which uses current file size - Now properly passes map_size as two DWORDs (high/low 32-bit parts) - Ensures mapping size matches requested size instead of file size - Fixes alloc_size mismatch when mapping larger than file size These fixes ensure Windows behavior matches Unix implementation and prevents undefined behavior when operations are performed on invalid file states. Signed-off-by: Eduardo Silva <[email protected]>
Replace Unix-specific file descriptor check with platform-agnostic function.
The code previously used 'cf->fd > 0' which only works on Unix systems
where file descriptors are positive integers. On Windows, files use
HANDLE objects and cf->fd is not set, causing the check to fail even
when files are open.
Changed from:
if (cf->fd > 0) { ... }
To:
if (cio_file_native_is_open(cf)) { ... }
This makes the code work correctly on both Windows and Unix platforms,
as cio_file_native_is_open() uses the appropriate platform-specific
check (cf->fd != -1 on Unix, cf->backing_file != INVALID_HANDLE_VALUE
on Windows).
Also removed the fd value from the error message since it's not
meaningful on Windows.
Signed-off-by: Eduardo Silva <[email protected]>
- Add ctx field to cio_file struct for context-aware logging - Add auto_remap_warned and map_truncated_warned flags to track warnings - Add proper error checking for cio_file_native_unmap() in munmap_file() - Add error checking for munmap_file() and cio_file_native_close() in cio_file_down() - Add warning when read-only maps are truncated during mapping - Add early return in cio_file_sync() for unmapped chunks (nothing to sync) - Initialize new warning flags in cio_file_open() Signed-off-by: Eduardo Silva <[email protected]>
Reset map_truncated_warned flag in cio_file_native_unmap() for consistency with Windows implementation. Signed-off-by: Eduardo Silva <[email protected]>
- Fix cio_file_native_unmap() to check mapping state before file handle state (Windows allows unmapping even if file handle is closed) - Add error checking for CloseHandle() on mapping handle - Fix cio_file_native_map() to properly handle file size for read-only files - Prevent mapping beyond file size for read-only files - Auto-resize read-write files when map_size > file_size - Improve cio_file_native_delete() to properly unmap and close handles before deletion - Add warning messages when auto-cleaning resources during delete - Reset map_truncated_warned flag on unmap Signed-off-by: Eduardo Silva <[email protected]>
- Update delete tests to expect success with auto-cleanup - Change tests to verify proper resource cleanup (unmap and close) - Update test comments to reflect new expected behavior - Use public APIs (cio_chunk_is_up) for state verification instead of directly accessing native functions Signed-off-by: Eduardo Silva <[email protected]>
edsiper
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.