From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Lars Magne Ingebrigtsen Newsgroups: gmane.emacs.devel Subject: Re: trunk r113804: * decompress.c: Fix bugs with large buffers and weird inputs. Date: Tue, 13 Aug 2013 18:21:09 +0200 Message-ID: References: <520A4ABC.2000204@cs.ucla.edu> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1376410901 10120 80.91.229.3 (13 Aug 2013 16:21:41 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 13 Aug 2013 16:21:41 +0000 (UTC) Cc: emacs-devel@gnu.org To: Paul Eggert Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Aug 13 18:21:43 2013 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1V9HLo-0007qt-NK for ged-emacs-devel@m.gmane.org; Tue, 13 Aug 2013 18:21:40 +0200 Original-Received: from localhost ([::1]:54756 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9HLo-0003XW-9X for ged-emacs-devel@m.gmane.org; Tue, 13 Aug 2013 12:21:40 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:49829) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9HLf-0003Ng-CV for emacs-devel@gnu.org; Tue, 13 Aug 2013 12:21:37 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V9HLY-0005ca-LJ for emacs-devel@gnu.org; Tue, 13 Aug 2013 12:21:31 -0400 Original-Received: from hermes.netfonds.no ([80.91.224.195]:52764) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V9HLY-0005bn-Dz for emacs-devel@gnu.org; Tue, 13 Aug 2013 12:21:24 -0400 Original-Received: from cm-84.215.51.58.getinternet.no ([84.215.51.58] helo=stories.gnus.org) by hermes.netfonds.no with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1V9HLK-00025Z-0M; Tue, 13 Aug 2013 18:21:10 +0200 Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAFVBMVEViUlj///X+/vbz7+T+ /fIYDRP///c7F+YMAAABzElEQVQ4jXWTW47bMAxFBRheABuB/2mBfgewZwGqBH0bGGQBTATtfwm9 JGUnmckoyEvHl2+G8uUkosLUJNi/eNwy9U7cpTvAU4AUiQWgM95BH1M5pcTELHrZHcSEV4mEs98a SGcHxQA9QD2TupBEYgrmXaGW8zLOmuuuIOosFttieB0gWXQ5HOcJQJHn6zjbDqr6ujnYXgAsCd0V bAGfWxngX8e9nHYwHyDxWYhOs5MQDlBgqaRs4DrPAHWPihWEB0hWSAWtlAFgbNNURx6tvYLehiLG sicIkGN9A65QUIoe1Q0dfwPQUHTJwawgcxqmfrfSDISLZp6pAEhI1JsmCDAtMxRrdFPIs0UDWhAo lqJAXsEFiqVKQd8NNIsqLAucoFYF9XPw18A2QMLIDMXII1ijVkiIRBvVijcqWFQ2y9xc4cMwAOuw UqjSY9mjsp6T6JBb2aXen0AC4Oimkvr41KAURGoMU/0mnf9oHhMyV5AoqkKQP99NMV0MwHWP4YyB o+qmpg8F8E4+cL3KCQWckDm+dIGs7JirKi0/9tkXVMNFeCddDNucvA5Qu6Bkv/rXo3lw55c7n8T6 7dmfgZuyDX4HhuJgt2/gWYbf/wHKajKHWXbQWgAAAABJRU5ErkJggg== X-Now-Playing: =?iso-8859-1?Q?=B5-Ziq's?= _Somerset Avenue Tracks (1992-1995) (1)_: "Air" X-Hashcash: 1:23:130813:emacs-devel@gnu.org::6PCwEtM2WBlra4nE:000000000000000000000000000000000000000000RSpE X-Hashcash: 1:23:130813:eggert@cs.ucla.edu::AKcaJiF4HMDmZZGT:0000000000000000000000000000000000000000000qgG5 In-Reply-To: (Lars Magne Ingebrigtsen's message of "Tue, 13 Aug 2013 17:10:29 +0200") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3.50 (gnu/linux) X-MailScanner-ID: 1V9HLK-00025Z-0M MailScanner-NULL-Check: 1377015670.3923@TmrkuguiCJbSHcMY3/HOZw X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 80.91.224.195 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:162666 Archived-At: Lars Magne Ingebrigtsen writes: > I think writing 1 << 14 and then checking whether that's larger than > UINT_MAX is still pretty unclear. My first thought, at least, was > "geez, he's making a HUGE buffer gap", until I started doing the math. Here's the current code, abbreviated: /* Maximum number of bytes that one 'inflate' call should read and write. Do not make avail_out too large, as that might unduly delay C-g. In any case zlib requires that these values not exceed UINT_MAX. */ enum { avail_out = 1 << 14 }; verify (avail_out <= UINT_MAX); ptrdiff_t decompressed; if (GAP_SIZE < avail_out) make_gap (avail_out - GAP_SIZE); stream.avail_out = avail_out; decompressed = avail_out - stream.avail_out; I think it's pretty opaque. What's with all the ptrdiff_t's when inflate only takes ints, anyway? And checking whether a constant is larger than UINT_MAX is the thing that makes me go "wha? Is there something clever going on here that I don't understand?" So why not do it simple and nice: /* Maximum number of bytes that one 'inflate' call should read and write. Do not make buffer_size too large, as that might unduly delay C-g. */ int decompressed, buffer_size = 16384; if (GAP_SIZE < buffer_size) make_gap (buffer_size - GAP_SIZE); stream.avail_out = buffer_size; decompressed = buffer_size - stream.avail_out; No odd constants, checks or explanations necessary. Although this may violate the new apparent dictum that at least every three lines need to compare with UINT_MAX, which seems to be the coding standard lately. >"? -- (domestic pets only, the antidote for overdose, milk.) No Gnus T-Shirt for sale: http://ingebrigtsen.no/no.php and http://lars.ingebrigtsen.no/2013/08/twenty-years-of-september.html