unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Lars Magne Ingebrigtsen <larsi@gnus.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: emacs-devel@gnu.org
Subject: Re: Mysterious gzipped images
Date: Thu, 08 Aug 2013 16:06:01 +0200	[thread overview]
Message-ID: <m3txj0tgza.fsf@stories.gnus.org> (raw)
In-Reply-To: <52039B84.4070603@cs.ucla.edu> (Paul Eggert's message of "Thu, 08 Aug 2013 06:22:12 -0700")

Paul Eggert <eggert@cs.ucla.edu> writes:

>>   out = (char *) malloc (BUFFER_SIZE);
>
> BUFFER_SIZE is so small that you should just put the buffer on the
> stack "char out[BUFFER_SIZE];".  That way, you don't need to worry
> about freeing it later (e.g., on error).

Ah, I thought having (largish) arrays on the stack was frowned upon, but
perhaps 16K isn't big these days?

> Or better yet, why not use the gap as the output buffer?
> That should avoid some unnecessary copying.

The buffer gap?  Ah, I see.  That's very clever.  I'll rework the code
to do that.

>>   stream.avail_in = iend - istart;
>
> On 64-bit platforms, this won't work on buffers larger than 4 GiB.
> I suggest 'stream.avail_in = min (iend - istart, UINT_MAX);'
> and then put the whole thing inside a loop that repeats
> until the input buffer is exhausted.

Right.

> Each time through the inner loop, it should QUIT so that the user can
> interrupt.  You need a record_unwind_protect around the whole thing;
> the unwind-protect should be the code that deletes any uncompressed
> data already inserted and restores point.

Hm...  is that necessary?  gunzipping is quite fast.  And if you have a
8GB compressed file somehow in your buffer, I would assume that we're in
the year 2025, and your machine is fast enough to make that not matter. >"?

On the other hand, if your machine starts thrashing because it's running
out of memory, having `C-g' work would be nice.  In which case, having
the outer loop be smaller might be a good idea.  Say, only decompress a
few hundred megs at a time, max, for instance.

>>     case Z_STREAM_ERROR:
>>     case Z_NEED_DICT:
>>     case Z_DATA_ERROR:
>>     case Z_MEM_ERROR:
>
> Shouldn't an error be signaled for this sort of thing?
> That would fit in with the unwind-protect.

Yeah, I debated with myself whether signalling an error or returning nil
would be the best interface here.  We have lots of decoding functions
that won't signal errors on invalid inputs (like
`rfc2047-decode-encoded-words'), because we're typically calling them in
tight loops, and slapping `condition-case' around those calls are
annoying.

I.e., they're not anywhere close to the user level, so making them
signal an error is almost never what we want.

Decompressing the region, on the other hand, won't usually be called
that way (in a loop), but it's not a user-level command, either.  So I
don't know.

-- 
(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



  reply	other threads:[~2013-08-08 14:06 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-06 22:20 Mysterious gzipped images Lars Magne Ingebrigtsen
2013-08-06 22:30 ` Lars Magne Ingebrigtsen
2013-08-06 23:05   ` Andreas Schwab
2013-08-06 23:38     ` Lars Magne Ingebrigtsen
2013-08-07  1:08       ` Lars Magne Ingebrigtsen
2013-08-07  8:51         ` Julien Danjou
2013-08-07 14:35         ` Stefan Monnier
2013-08-07 16:47           ` chad
2013-08-08 10:35           ` Lars Magne Ingebrigtsen
2013-08-08 10:57             ` Andreas Schwab
2013-08-11 19:44               ` Lars Magne Ingebrigtsen
2013-08-08 13:22             ` Paul Eggert
2013-08-08 14:06               ` Lars Magne Ingebrigtsen [this message]
2013-08-08 15:31                 ` Lars Magne Ingebrigtsen
2013-08-11 22:05                   ` Lars Magne Ingebrigtsen
2013-08-08 13:24             ` Romain Francoise
2013-08-08 13:28               ` Lars Magne Ingebrigtsen
2013-08-08 13:36                 ` Christopher Schmidt
2013-08-08 13:55                   ` Lars Magne Ingebrigtsen
2013-08-08 14:06                     ` Christopher Schmidt
2013-08-08 14:09                       ` Lars Magne Ingebrigtsen
2013-08-08 15:43                 ` Romain Francoise
2013-08-08 15:52                   ` Lars Magne Ingebrigtsen
2013-08-11 19:51                   ` Lars Magne Ingebrigtsen
2013-08-08 14:47             ` Stefan Monnier
2013-08-11 19:47               ` Lars Magne Ingebrigtsen
2013-08-12  0:54                 ` Stefan Monnier
2013-08-12 14:05                   ` Lars Magne Ingebrigtsen
2013-08-12 15:09                     ` Stefan Monnier
2013-08-12 15:20                     ` Eli Zaretskii
2013-08-12 16:27                       ` Lars Magne Ingebrigtsen
2013-08-12 16:42                         ` Eli Zaretskii
2013-08-12 12:13                 ` Eli Zaretskii
2013-08-12 12:21                   ` Eli Zaretskii
2013-08-12 13:06                     ` Lars Magne Ingebrigtsen
2013-08-12 13:29                       ` Eli Zaretskii
2013-08-13 23:40                       ` Lars Magne Ingebrigtsen
2013-08-14 13:09                         ` Lars Magne Ingebrigtsen
2013-08-14 15:12                           ` Eli Zaretskii
2013-08-17 15:01                             ` Lars Magne Ingebrigtsen
2013-08-17 15:27                               ` Eli Zaretskii
2013-08-17 17:04                                 ` Eli Zaretskii
2013-08-18 17:06                                   ` Lars Magne Ingebrigtsen
2013-08-07 18:25         ` Rüdiger Sonderfeld
2013-08-07 21:06           ` FFI (was: Mysterious gzipped images) Stefan Monnier
2013-08-08  0:50             ` Stephen J. Turnbull

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m3txj0tgza.fsf@stories.gnus.org \
    --to=larsi@gnus.org \
    --cc=eggert@cs.ucla.edu \
    --cc=emacs-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).