Re: -Wstringop-overflow warning on compiling info

On Thu, Jan 22, 2026 at 07:26:48PM +0000, Gavin Smith wrote:
> This warning is issued with the current git master as well as Texinfo 7.2.
> The warning message from gcc is very obscure but appears to be about
> the following line of code:
> 
>       if ((read (descriptor, contents, read_file_size)) != read_file_size)
> 
> It does not like the third argument to 'read'.
> 
> I found reverting the following change got rid of the warning:
> 
> Author: Patrice Dumas <[email protected]>
> Date:   2024-10-10 11:56:24 +0200
> 
>     * info/filesys.c (filesys_read_info_file): convert to ssize_t to use
>     as read return value an not to size_t and convert to size_t later on.
>     Do not cast finfo->st_size to (long), leave it as off_t and convert to
>     either size_t or ssize_t depending on how the file is read.  Add
>     comments to mark conversion from unsigned to signed.
>     
>     * info/filesys.c (convert_eols): use size_t in argument an as return
>     type.
> 
> I found changing a single line would also get rid of the warning:
> 
> --- a/info/filesys.c
> +++ b/info/filesys.c
> @@ -330,7 +330,7 @@ filesys_read_info_file (char *pathname, size_t *filesize,
>    else
>      {
>        int descriptor;
> -      ssize_t read_file_size = stat_fsize;
> +      size_t read_file_size = stat_fsize;
>  
>        *is_compressed = 0;
>        descriptor = open (pathname, O_RDONLY | O_BINARY, 0666);
> 
> 
> So does adding a check that read_file_size is positive:
> 
> --- a/info/filesys.c
> +++ b/info/filesys.c
> @@ -344,7 +344,8 @@ filesys_read_info_file (char *pathname, size_t *filesize,
>  
>        /* Try to read the contents of this file. */
>        contents = xmalloc (1 + read_file_size);
> -      if ((read (descriptor, contents, read_file_size)) != read_file_size)
> +      if (read_file_size > 0
> +        && (read (descriptor, contents, read_file_size)) != read_file_size)
>          {
>           filesys_error_number = errno;
>           close (descriptor);
> 
> The third argument to 'read' is declared as size_t so perhaps the warning
> is triggered by passing in a value of type ssize_t that could potentially
> be negative.
> 
> I'll try to commit a fix to add such a check.
> 
> I haven't understand fully all the reasons for the changes to types in
> this code.  I expect this problem is a hangover from people deciding to
> convert from unsigned integer types to signed integer types or vice versa.
> 
> (In commit 219bed49caf262a (2012-11-17), types were changed from long (signed)
> to size_t (unsigned).  This was further changed in the commit above.)

I can say for the changes I made.  The idea was to avoid mixing signed
and unsigned integers because it can easily lead to bugs because going
through 0 may get unnoticed, and also probably because it is undefined
behaviour.  When this is anavoidable, at least add something that shows
that a conversion is needed or such.  There is a warning that helps to
make the issue visible:

warning: comparison of integer expressions of different signedness: ‘long int’ and ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]

If I recall well, I also tried to avoid or minimize the possibility to
have integer overflows.

I also remember that sometimes it was easy to change the type, and it
could make the code more readable, by making unambiguous that a negative
value could never be possible.  But sometimes it was not possible to do
something clean.

-- 
Pat