chunkio: replace Windows API PathIsDirectoryA for nano server compatibility #10734
Conversation
…bility Replace Windows API call PathIsDirectoryA that is not available on Windows Nano Server with compatible alternatives. The changes maintain full functionality while using APIs that are available across all Windows variants including Nano Server. Signed-off-by: zanejohnson-azure <zanejohnson@microsoft.com>
zanejohnson-azure
requested review from
edsiper,
fujimotos and
koleini
as code owners
WalkthroughThe Windows-specific dirent implementation updates opendir to use GetFileAttributesA for directory detection instead of PathIsDirectoryA, removing the shlwapi dependency. It adds a local DWORD for attributes and returns NULL when attributes are invalid or the directory flag is absent. No public APIs changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
lib/chunkio/src/win32/dirent.c (2)
33-33: Fix ANSI/Wide API mismatch to avoid build/runtime issuesThe file mixes A-suffixed APIs with generic types/functions:
- WIN32_FIND_DATA (generic) is paired with FindFirstFileA.
- FindNextFile (generic) is used in readdir, while FindFirstFileA is used in opendir.
If UNICODE is defined, these mismatches will break compilation or lead to type inconsistencies. Use A-suffixed types/APIs consistently.
Apply this diff:
- WIN32_FIND_DATA find_data; + WIN32_FIND_DATAA find_data;- if (FindNextFile(d->h, &d->find_data) == 0) { + if (FindNextFileA(d->h, &d->find_data) == 0) {Also applies to: 115-116
58-75: Prevent out-of-bounds read on empty path and accept both slash typescreate_pattern reads path[len - 1] without guarding len > 0, which dereferences before the buffer for empty strings. Also, Windows accepts both '' and '/' as separators.
Apply this diff:
- size_t len = strlen(path); + size_t len = strlen(path); size_t buflen = len + 3; buf = malloc(buflen); if (buf == NULL) { return NULL; } strcpy_s(buf, buflen, path); - if (path[len - 1] == '\\') { + if (len == 0) { + /* Empty path is invalid for opendir semantics */ + free(buf); + return NULL; + } + + /* Accept both backslash and forward slash */ + if (path[len - 1] == '\\' || path[len - 1] == '/') { strcat_s(buf, buflen, "*"); } else { strcat_s(buf, buflen, "\\*"); } return buf;
🧹 Nitpick comments (4)
lib/chunkio/src/win32/dirent.c (4)
40-50: Use DWORD for file attribute parameter typeget_filetype takes an int but receives Win32 file attributes (DWORD). Use DWORD to avoid implicit narrowing and better reflect API types.
Apply this diff:
-static unsigned char get_filetype(int dwFileAttributes) +static unsigned char get_filetype(DWORD dwFileAttributes) { if (dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { return DT_DIR; } else if (dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) { return DT_LNK; } return DT_REG; }No call-site changes are needed since d->find_data.dwFileAttributes is already a DWORD.
Also applies to: 122-123
25-27: Include standard headers explicitlymalloc/calloc/free/strlen/strcpy_s/strcat_s are used but <stdlib.h> and <string.h> are not included here. Include them to ensure prototypes and avoid implicit declarations across compilers.
Apply this diff:
#include <Windows.h> #include "dirent.h" +#include <stdlib.h> +#include <string.h>
21-23: Typo in comment (“FIndFirstFile”)Minor spelling fix.
Apply this diff:
- * Win32's FIndFirstFile/FindNextFile API. + * Win32's FindFirstFile/FindNextFile API.
38-40: Typo in comment (“flle” -> “file”)Minor spelling fix.
Apply this diff:
- * Guess POSIX flle type from Win32 file attributes. + * Guess POSIX file type from Win32 file attributes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/chunkio/src/win32/dirent.c(1 hunks)
🔇 Additional comments (1)
lib/chunkio/src/win32/dirent.c (1)
81-83: Nano Server–compatible directory check: LGTMSwitching to GetFileAttributesA removes the shlwapi dependency and aligns with Nano Server constraints while preserving behavior. Good change.
|
would you please submit this PR to fluent/chunkio repo ? |
|
This is already done on fluent/chunkio#109. We need to sync the latest chunkio from its upstream. |
Replace Windows API calls in chunkio dirent.c with Nano Server compatible alternatives. The original APIs are not available in Windows Nano Server, preventing Fluent Bit from running in minimal Windows environments and containers.
This change maintains full functionality while ensuring compatibility across all Windows variants including Nano Server deployments.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit