Re: [PATCH v2 5/5] convert: add filter.<driver>.process option

On 05 Aug 2016, at 20:55, Eric Wong [off-list ref] wrote:

Lars Schneider [off-list ref] wrote:

quoted

quoted

On 27 Jul 2016, at 11:41, Eric Wong [off-list ref] wrote:
larsxschneider@gmail.com wrote:

quoted

+static int apply_protocol_filter(const char *path, const char *src, size_t len,
+						int fd, struct strbuf *dst, const char *cmd,
+						const char *filter_type)
+{

<snip>

quoted

+	if (fd >= 0 && !src) {
+		ret &= fstat(fd, &file_stat) != -1;
+		len = file_stat.st_size;

Same truncation bug I noticed earlier; what I originally meant
is the `len' arg probably ought to be off_t, here, not size_t.
32-bit x86 Linux systems have 32-bit size_t (unsigned), but
large file support means off_t is 64-bits (signed).

OK. Would it be OK to keep size_t for this patch series?

I think there should at least be a truncation warning (or die)
for larger-than-4GB files on 32-bit.  I don't know how common
they are for git-lfs users.

Perhaps using xsize_t in git-compat-util.h works for now:

	len = xsize_t(file_stat.st_size);

Thanks for the hint! Should I add the same check to sha1_file's use
of fstat in line 1002 or is it not needed there?

https://github.com/git/git/blob/c6b0597e9ac7277e148e2fd4d7615ac6e0bfb661/sha1_file.c#L1002

Thanks,
Lars