emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: warn-maybe-out-of-memory


From: Eli Zaretskii
Subject: Re: warn-maybe-out-of-memory
Date: Fri, 11 Jul 2014 09:50:03 +0300

> Date: Fri, 11 Jul 2014 08:42:23 +0400
> From: Dmitry Antipov <address@hidden>
> CC: address@hidden
> 
> On 07/10/2014 10:47 PM, Eli Zaretskii wrote:
> 
> >> I wonder if this function should only warn when it is called from
> >> commands invoked by the user, as opposed to from a Lisp program.  The
> >> warning is in find-file-noselect, which AFAIK is widely used in Lisp
> >> programs, where displaying this warning might be inappropriate.
> 
> Hm, find-file-noselect also may ask for confirmation to open large file.
> If this is undesirable, shouldn't we move all user interaction to top-level
> find-file?

Maybe.

> >> In addition, the function assumes that visiting a file of size N bytes
> >> needs N bytes of memory, which is false: we need more, sometimes much
> >> more.
> 
> No.  This function assumes that visiting a file of size N bytes needs
> at least N bytes of memory, and warns if we have even less than N.

But if this warning is to be useful, it should catch the majority of
the cases, otherwise we are just wasting cycles.  With the current
test, many cases of visiting files that cannot be accommodated will go
undetected, thus rendering all this feature useless.

Since it's only a warning that doesn't prevent visiting a file, it is
better to err on the other side of the truth.  E.g., apply some
heuristics as to the average expansion factor, perhaps dependent on
the locale.  And don't forget that even for plain-ASCII files we
allocate the gap in addition to the text itself, so the mere size of
the file is simply _never_ the correct value, it is always an
underestimation.  IOW, this test is always biased towards lower
values.

If we don't do something like that, then I wonder what exactly is the
use case that benefits from this warning?

> > Also, is this call to emacs_abort really appropriate?  Or is it some
> > remnant from debugging this code?
> >
> >      if (sysinfo (&si))
> >        emacs_abort ();  <<<<<<<<<<<<<<<<<<<<<<
> 
> Here emacs_abort may be called only if &SI points outside of a process'
> address space.  This is possible only if C stack is smashed and so you
> have no way to continue anyway.

It is IMO inappropriate for a minor feature like this one to abort
Emacs.  Who knows what the Linux kernel developers will introduce
tomorrow that could possibly fail the function?  It is none of this
feature's business to be the place where C stack smashing is detected.
So my vote is for returning nil and perhaps displaying a warning.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]