Hi Neeraj, On Mon, 6 Dec 2021, Neeraj Singh wrote: > I'm a little confused by this issue. Looks like an empty flex array > is supported here: https://godbolt.org/z/j3ndYTEfx. It is somewhat strange, but understandable, that the empty flex array at the end of a struct isn't triggering a compile error. But having a field _after_ that empty flex array _does_ trigger a compile error: struct MyStruct { int x; int flexA[]; char string[256]; }; Note the added `string` field. See https://godbolt.org/z/TbcYhEW5d, it says: <source>(5): error C2229: struct 'MyStruct' has an illegal zero-sized array Compiler returned: 2 > (Note that I'm passing /TC to force C compilation). Yes, `/TC` is one of the flags we pass to MSVC. For more details, see https://github.com/git-for-windows/git/runs/4389081542?check_suite_focus=true#step:9:125 > Also, heap_fsentry doesn't appear to use a flex array in current sources. It does, but it is admittedly a bit convoluted and not very easy to see. The definition of `heap_fsentry` is [this](https://github.com/git-for-windows/git/blob/v2.34.1.windows.1/compat/win32/fscache.c#L77-L80): struct heap_fsentry { struct fsentry ent; char dummy[MAX_LONG_PATH]; }; No flex array here, right? But wait, there is a `struct fsentry`. Let's look at [that](https://github.com/git-for-windows/git/blob/v2.34.1.windows.1/compat/win32/fscache.c#L43-L74): struct fsentry { struct hashmap_entry ent; [...] /* * Name of the entry. For directory listings: relative path of the * directory, without trailing '/' (empty for cwd()). For file * entries: * name of the file. Typically points to the end of the structure * if * the fsentry is allocated on the heap (see fsentry_alloc), or to * a * local variable if on the stack (see fsentry_init). */ struct dirent dirent; }; Still no flex array, right? But wait, there is a `struct dirent`. Let's [see](https://github.com/git-for-windows/git/blob/v2.34.1.windows.1/compat/win32/dirent.h#L9-L12): struct dirent { unsigned char d_type; /* file type to prevent lstat after readdir */ char d_name[FLEX_ARRAY]; /* file name */ }; Finally! We see the flex array. Now, you may ask why is this even correct? How can you have an empty-sized field in a struct that is inside another struct that is inside yet another struct _and then followed by another field_? The reason why this is correct and intended is that `struct dirent` intentionally leaves the length of the `d_name` undefined, to leave it to the implementation whether a fixed-size buffer is used or a specifically-allocated one of the exact correct size for a _specific_ directory entry. In FSCache, we want to allocate a large-enough buffer to fit _any_ file name, and it should not only contain the metadata in `struct dirent`, but additionally some FSCache-specific metadata. Therefore, `struct fsentry` is kind of a subclass of `struct dirent`, and `struct heap_fsentry` is kind of a subclass of something that does not exist, a `struct dirent` that offers enough space to fit _any_ legal `d_name` (that is what that `dummy` field is for, it is not actually intended to be accessed except via `d_name`). > If it does start using it, there issue may actually be elsewhere besides > the struct definition (it could be just a badly targeted compiler error). > We have code like `struct heap_fsentry key[2];`. That declaration can't > work with a flex array. I hope my explanation above made sense to you. Admittedly, it is slightly icky code, but honestly, I do not have any splendid idea how to make it less complicated to understand. Do you? Ciao, Dscho
