Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxx> writes: > 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. Ah, sorry, so then I misread your comment. size_to_blocks() in compat/cygwin.c counts blocks in 512 (I just checked) and you are applying the reverse. But you are right. If the emulation used on cygwin is _not_ doing the "blocks * blksize is close to size" thing that is not POSIX but you saw in your experiment on NTFS, and if we need your follow-up patch to make it do so, there is no point in using my patch. > 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. Thanks for a clarification. I've already queued both to 'next', but I will revert mine and make your patch graduate to 'master'. -- 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