Re: Patch: fix various issues found by static analysis [LONG_MAX]

[Gavin Smith]

On Tue, Oct 15, 2024 at 12:36:59PM +0200, Vitezslav Crhonek wrote:
> Hi,
> 
> Please review proposed fixes of issues found by static analysis
> of texinfo-7.1 in the attached patch and consider applying them.

Thanks for sending them, I would like to take some time to look
at and understand them.

> From e807d95e3422b1b45b6ec9d3b6b0f559c136fa5f Mon Sep 17 00:00:00 2001
> From: Vitezslav Crhonek <vcrhonek@redhat.com>
> Date: Tue, 15 Oct 2024 10:51:07 +0200
> Subject: [PATCH 1/7] * info/makedoc.c: fix possible integer overflow
> 
> ---
>  info/makedoc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/info/makedoc.c b/info/makedoc.c
> index 6810d4d228..9f525df110 100644
> --- a/info/makedoc.c
> +++ b/info/makedoc.c
> @@ -296,7 +296,7 @@ process_one_file (char *filename, FILE *doc_stream, FILE 
> *funs_stream)
>        char *func, *doc;
>        char *func_name;
>  
> -      for (; offset < (file_size - decl_len); offset++)
> +      for (; offset < (file_size - decl_len) && offset < (LONG_MAX - 
> decl_len); offset++)
>          {
>            if (buffer[offset] == '\n')
>              {

Do we really need to check for integer overflow?  Surely every C program
in existence has possible integer overflows if its input gets big enough
and I don't want comparisons to LONG_MAX etc. everywhere.

The check 

   offset < (file_size - decl_len) && offset < (LONG_MAX - decl_len)

is strange.  One of the two values that offset is compared to is as least
as large as the other (and files_size and decl_len don't change values
throughout the loop).

Suppose

   (file_size - decl_len) > (LONG_MAX - decl_len)

Then 

   file_size > LONG_MAX

This is impossible as file_size is a long.

So (file_size - decl_len) <= (LONG_MAX - decl_len) and the part you
added is unnecessary.