From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Preview: portable dumper Date: Sat, 17 Feb 2018 10:54:15 +0200 Message-ID: <83a7w8c7yg.fsf@gnu.org> References: <1775923222.898447.1518559575706@mail.libero.it> <277032065.898859.1518560883271@mail.libero.it> <1456022824.1085831.1518815658356@mail.libero.it> <46535781-d5c2-fee8-05aa-37a29988ce86@dancol.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1518857564 7155 195.159.176.226 (17 Feb 2018 08:52:44 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 17 Feb 2018 08:52:44 +0000 (UTC) Cc: emacs-devel@gnu.org To: Daniel Colascione Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Feb 17 09:52:39 2018 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1emyEG-0000UO-J2 for ged-emacs-devel@m.gmane.org; Sat, 17 Feb 2018 09:52:20 +0100 Original-Received: from localhost ([::1]:54207 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emyGI-0008Dh-Lk for ged-emacs-devel@m.gmane.org; Sat, 17 Feb 2018 03:54:26 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35161) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emyGB-0008DA-SA for emacs-devel@gnu.org; Sat, 17 Feb 2018 03:54:21 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1emyG6-0004yj-VK for emacs-devel@gnu.org; Sat, 17 Feb 2018 03:54:19 -0500 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:50383) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emyG6-0004yT-RV; Sat, 17 Feb 2018 03:54:14 -0500 Original-Received: from [176.228.60.248] (port=3208 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1emyG6-0000HS-5q; Sat, 17 Feb 2018 03:54:14 -0500 In-reply-to: <46535781-d5c2-fee8-05aa-37a29988ce86@dancol.org> (message from Daniel Colascione on Fri, 16 Feb 2018 13:25:00 -0800) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:222854 Archived-At: > From: Daniel Colascione > Date: Fri, 16 Feb 2018 13:25:00 -0800 > > >> pdumper.c: In function 'dump_read_all': > >> pdumper.c:4723: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); > >> ^~~~~~~~~~~~~ > >> CC data.o > >> cc1.exe: some warnings being treated as errors > > > > I have seen you have commit many fixes but here the build still fails in the same manner.. Maybe you have not found the right solution yet.. > > On this one, I was waiting to see how the broader discussion went. OK, I've looked at the code, both in sysdep.c and in pdumper.c. My conclusion is that there's no real problem in sysdep.c, due to these comments: /* Maximum number of bytes to read or write in a single system call. This works around a serious bug in Linux kernels before 2.6.16; see . It's likely to work around similar bugs in other operating systems, so do it on all platforms. Round INT_MAX down to a page size, with the conservative assumption that page sizes are at most 2**18 bytes (any kernel with a page size larger than that shouldn't have the bug). */ #ifndef MAX_RW_COUNT #define MAX_RW_COUNT (INT_MAX >> 18 << 18) #endif /* Read from FD to a buffer BUF with size NBYTE. If interrupted, process any quits and pending signals immediately if INTERRUPTIBLE, and then retry the read unless quitting. Return the number of bytes read, which might be less than NBYTE. On error, set errno to a value other than EINTR, and return -1. */ static ptrdiff_t emacs_intr_read (int fd, void *buf, ptrdiff_t nbyte, bool interruptible) { ssize_t result; /* There is no need to check against MAX_RW_COUNT, since no caller ever passes a size that large to emacs_read. */ do { if (interruptible) maybe_quit (); result = read (fd, buf, nbyte); } while (result < 0 && errno == EINTR); Since MAX_RW_COUNT is less than INT_MAX, we have no real problem here for MS-Windows, as 'nbyte' will never overflow an unsigned int. We could add an eassert there, for Windows only, though, to make this assumption explicit. As for pdumper, we could do one of 2 things: . make a reasonable assumption that no .pdmp file will ever be larger than 2GB, change the assertion there which checks against SSIZE_MAX to check against UINT_MAX instead, and work internally with unsigned int instead of size_t counts; or . write a separate Windows-specific version of dump_read_all, which in the 64-bit build would limit to UINT_MAX the number of bytes we read on each iteration through the loop. Daniel, which of these 2 alternatives do you prefer? Or does anyone have a better proposal? (I already considered rewriting the code in w32.c:sys_read so that it accepts a size_t last argument, and decided it was unjustified for this single caller, as all the other direct calls to 'read' in Emacs either use fixed small byte counts, or are not compiled in the Windows build.) Thanks.