Re: [PATCH 3/7] builtin/bugreport.c: avoid size_t overflow

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Mon, Aug 01 2022, Victoria Dye via GitGitGadget wrote:

> From: Victoria Dye <vdye@github.com>
>
> Avoid size_t overflow when reporting the available disk space in
> 'get_disk_info' by casting the block size and available block count to
> 'uint64_t' before multiplying them. Without this change, 'st_mult' would
> (correctly) report size_t overflow on 32-bit systems at or exceeding 2^32
> bytes of available space.
>
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
>  builtin/bugreport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> index 35b1fc48bf1..720889a37ad 100644
> --- a/builtin/bugreport.c
> +++ b/builtin/bugreport.c
> @@ -258,7 +258,7 @@ static int get_disk_info(struct strbuf *out)
>  	}
>  
>  	strbuf_addf(out, "Available space on '%s': ", buf.buf);
> -	strbuf_humanise_bytes(out, st_mult(stat.f_bsize, stat.f_bavail));
> +	strbuf_humanise_bytes(out, (uint64_t)stat.f_bsize * (uint64_t)stat.f_bavail);

Doesn't this remove the overflow guard on 64 bit systems to support
those 32 bit systems?

I also don't tthink it's correct that this would "correctly
report...". Before this we were simply assuming that "size_t" and
"unsigned long" & "fsblkcnt_t" would all yield the same thing.

But I don't think per [1] and [2] that POSIX is giving us any guarantees
in that regard, even on 32 bit systems, but perhaps it's a reasonable
assumption in practice.

1. https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/statvfs.h.html
2. https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html