Re: [PATCH] git-compat-util(msvc): C11 does not imply support for zero-sized arrays

Hi Neeraj,

On Mon, 6 Dec 2021, Neeraj Singh wrote:

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

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

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`).

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