long_to_cons doesn't always return a cons

long_to_cons doesn't always return a cons

[Alan Donovan]

This bug report will be sent to the Free Software Foundation,
not to your local site managers!
Please write in English, because the Emacs maintainers do not have
translators to read other languages for them.

Your bug report will be posted to the bug-gnu-emacs@gnu.org mailing list,
and to the gnu.emacs.bug news group.

In GNU Emacs 21.4.1 (i486-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2006-05-17 on rothera, modified by Debian
configured using `configure '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--with-x=yes' '--with-x-toolkit=athena' 'CFLAGS=-DDEBIAN -g -O2' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu''
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: C
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: C
  value of $LC_NUMERIC: C
  value of $LC_TIME: C
  value of $LANG: en_US
  locale-coding-system: iso-latin-1
  default-enable-multibyte-characters: t

Please describe exactly what actions triggered the bug
and the precise symptoms of the bug:


Summary: long_to_cons (in data.c) doesn't always return a cons, but it
should.

The symptom of the problem: using dired to navigate a filesystem in
which some directories have mtime=1 sometimes fails with "Wrong type
argument: listp, 1".

Gory details: the filesystem returns 1 for the mtime of a particular
directory.  The function `dired-internal-noselect' calls
`visited-file-modtime' on the dired buffer for the directory, which
returns not a pair (as it claims) but a scalar number, in this case 1.
dired-internal-noselect then applies car to this value, causing the
type error.

visited-file-modtime claims to return a pair of integers (HI . LO)
containing the high and low 16-bit words of the time_t.  It also
claims that the format is the same as for `file-attributes', which
also mentions the pair of integers, though not the possibility of
returning a scalar.  (Incidentally, visited-file-modtime explicitly
checks for the case where a scalar zero is encountered--clearly, this
situation isn't uncommon--but not for a scalar 1 such as returned in
this obscure case.)

Looking at the C source for visited-file-modtime, and for
long_to_cons, which it calls, it's clear that both can return scalar
numbers, though the docstring for neither mentions this:

  DEFUN ("visited-file-modtime", Fvisited_file_modtime,
         Svisited_file_modtime, 0, 0, 0,
         doc: /* Return the current buffer's recorded visited file
         modification time.
  The value is a list of the form (HIGH LOW), like the time values
  that `file-attributes' returns.  If the current buffer has no recorded
  file modification time, this function returns 0.
  See Info node `(elisp)Modification Time' for more details.  */)
       ()
  {
    Lisp_Object tcons;
    tcons = long_to_cons ((unsigned long) current_buffer->modtime);
    if (CONSP (tcons))
      return list2 (XCAR (tcons), XCDR (tcons));
    return tcons;
  }


  /* Convert between long values and pairs of Lisp integers.  */

  Lisp_Object
  long_to_cons (i)
       unsigned long i;
  {
    unsigned long top = i >> 16;
    unsigned int bot = i & 0xFFFF;
    if (top == 0)
      return make_number (bot);
    if (top == (unsigned long)-1 >> 16)
      return Fcons (make_number (-1), make_number (bot));
    return Fcons (make_number (top), make_number (bot));
  }

Furthermore, the info pages for file-attributes and
visited-file-modtime also don't mention the scalar case.

[Note that more recent versions of dired.el contain a workaround for
the underlynig problem--see
http://cvs.savannah.gnu.org/viewvc/emacs/emacs/lisp/dired.el?r1=1.291&r2=1.292]

It seems like the right fix here would be to change long_to_cons to
meet its specification, by making it return a cons (0 . n) instead of
a scalar number n.


cheers
alan

Re: long_to_cons doesn't always return a cons

[Richard Stallman]

Does this fix it?

*** fileio.c	22 Mar 2007 11:13:17 -0400	1.580
--- fileio.c	17 Jun 2007 10:56:38 -0400	
***************
*** 5692,5702 ****
  See Info node `(elisp)Modification Time' for more details.  */)
       ()
  {
!   Lisp_Object tcons;
!   tcons = long_to_cons ((unsigned long) current_buffer->modtime);
!   if (CONSP (tcons))
!     return list2 (XCAR (tcons), XCDR (tcons));
!   return tcons;
  }
  
  DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,
--- 5692,5700 ----
  See Info node `(elisp)Modification Time' for more details.  */)
       ()
  {
!   if (! current_buffer->modtime)
!     return make_number (0);
!   return make_time ((time_t) current_buffer->modtime);
  }
  
  DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,

Re: long_to_cons doesn't always return a cons

[Alan Donovan]

On Sun, Jun 17, 2007 at 05:49:12PM -0400, Richard Stallman wrote:
> Does this fix it?
>
> *** fileio.c	22 Mar 2007 11:13:17 -0400	1.580
> --- fileio.c	17 Jun 2007 10:56:38 -0400
> ***************
> *** 5692,5702 ****
>   See Info node `(elisp)Modification Time' for more details.  */)
>        ()
>   {
> !   Lisp_Object tcons;
> !   tcons = long_to_cons ((unsigned long) current_buffer->modtime);
> !   if (CONSP (tcons))
> !     return list2 (XCAR (tcons), XCDR (tcons));
> !   return tcons;
>   }
>
>   DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,
> --- 5692,5700 ----
>   See Info node `(elisp)Modification Time' for more details.  */)
>        ()
>   {
> !   if (! current_buffer->modtime)
> !     return make_number (0);
> !   return make_time ((time_t) current_buffer->modtime);
>   }
>
>   DEFUN ("set-visited-file-modtime", Fset_visited_file_modtime,

I confess I'm not building from CVS but using the emacs-snapshot
package instead, so I haven't tested this patch.  From the point of
view of visited-file-modtime, this change looks good to me.

Of course, long_to_cons still doesn't meet its spec in the case where
the top 16 bits are zero, so any other place where this function is
called might suffer from a similar problem.  If I were you, I would
additionally update the docstring for long-to-cons and cons-to-long to
mention the special treatment of values in this range.

cheers
alan