Re: Preview: portable dumper

[Eli Zaretskii]

> From: Andy Moreton <address@hidden>
> Date: Fri, 16 Feb 2018 11:33:35 +0000
> 
> On Thu 15 Feb 2018, Eli Zaretskii wrote:
> 
> C:/emacs/git/emacs/pdumper/src/pdumper.c: In function 'dump_read_all':
> C:/emacs/git/emacs/pdumper/src/pdumper.c:4784:45: error: conversion to 
> 'unsigned int' from 'size_t {aka long long unsigned int}' may alter its value 
> [-Werror=conversion]
>          read (fd, (char*) buf + bytes_read, bytes_to_read - bytes_read);
>                                              ^~~~~~~~~~~~~
> In file included from C:/emacs/git/emacs/pdumper/src/character.h:27:0,
>                  from C:/emacs/git/emacs/pdumper/src/buffer.h:27,
>                  from C:/emacs/git/emacs/pdumper/src/pdumper.c:18:
> C:/emacs/git/emacs/pdumper/src/pdumper.c: In function 'dump_object_1':
> C:/emacs/git/emacs/pdumper/src/lisp.h:206:5: warning: this statement may fall 
> through [-Wimplicit-fallthrough=]
>     (suppress_checking || (cond)     \
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      ? (void) 0       \
>      ~~~~~~~~~~~~~~~~~~
>      : die (# cond, __FILE__, __LINE__))
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> C:/emacs/git/emacs/pdumper/src/pdumper.c:2915:11: note: in expansion of macro 
> 'eassert'
>            eassert ("should not be dumping int: is self-representing" && 0);
>            ^~~~~~~
> C:/emacs/git/emacs/pdumper/src/pdumper.c:2916:9: note: here
>          default:
>          ^~~~~~~
> cc1.exe: some warnings being treated as errors
> 
> 
> This appears to be because sys_read() and _read() take an unsigned int
> for count rather than size_t.

Yes.  Angelo already reported this a few days ago.

> Changing the code to:
> 
> read (fd, (char*) buf + bytes_read, (int)(bytes_to_read - bytes_read));

This cannot be the right solution, because you will lose bits in the
cast, when bytes_to_read is greater than INT_MAX.

When Angelo reported this, I said I didn't understand how a similar
code in sysdep.c:emacs_intr_read does compile without a problem,
although it seems to feed a ptrdiff_t value (which should be 64-bit
wide in the 64-bit Windows build, just like size_t, except for the
signedness), similarly to the above.  Can you spot why that works?
Maybe look at the preprocessed source?  Maybe then we will know how to
fix that properly (or maybe we will discover one more subtle bug ;-).

In general, breaking the read into chunks of INT_MAX should work as
well, but I'd like to know first what's going on in emacs_intr_read,
because perhaps there's a more elegant solution we have available.