Junio C Hamano wrote:
> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes:
>
>> I haven't tried this patch, but I think you may need to add something like
>> the following (*not tested*):
>>
>> --- >8 ---
>> diff --git a/compat/cygwin.c b/compat/cygwin.c
>> index b4a51b9..7e9edec 100644
>> --- a/compat/cygwin.c
>> +++ b/compat/cygwin.c
>> @@ -53,6 +53,7 @@ static int do_stat(const char *file_name, struct stat *buf, stat_fn_t cygstat)
>> buf->st_size = (off_t)fdata.nFileSizeLow;
>> #endif
>> buf->st_blocks = size_to_blocks(buf->st_size);
>> + buf->st_blksize = 512;
>> filetime_to_timespec(&fdata.ftLastAccessTime, &buf->st_atim);
>> filetime_to_timespec(&fdata.ftLastWriteTime, &buf->st_mtim);
>> filetime_to_timespec(&fdata.ftCreationTime, &buf->st_ctim);
>> --- >8 ---
>
> Doesn't this contradict with everything you said?
Err... no :-D
Note that my suggested addition to your patch is in the core.filemode == false
code path, and so does not affect the "disk-space under-estimate" problem at all.
[To be clear: the "disk-space under-estimate" problem only happens when
core.filemode == true and the regular cygwin lstat()/stat() functions are used.
When core.filemode == false, the code in compat/cygwin.c (namely cygwin_lstat()
and cygwin_stat()) will (most likely) be called instead. These functions use
WIN32 api calls to implement equivalent, but presumably faster, versions of the
stat functions]
> You are forcing st_blksize to 512 but still return the same old st_blocks;
> I do not understand what that would achieve.
Well, as I said, I haven't tested your patch, or my suggested addition, so I could
well be wrong... but what I aimed to achieve was to:
- avoid "undefined behaviour" in on_disk_bytes(), since the value in
st_blksize would otherwise be undefined (ie whatever happened to be
on the stack-frame of the count_objects() function).
- initialize the st_blksize field with a value consistent with the
st_blocks field, which is derived from the st_size field, as the
number of 512-byte blocks. (see the context line just before the
+ line in the above diff, along with the size_to_blocks macro)
- return the same answer from this code as before.
Note that the answer returned from this core.filemode == false code path
is different to the core.filemode == true code path. Which is why I *slightly*
prefer my original patch.
HTH.
ATB,
Ramsay Jones
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html