GNU bug report logs - #2259
23.0.90; PATCH: getenv mistakenly passes a frame to getenv_internal, when it should instead pass a CONS

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 2259 in the body.
You can then email your comments to 2259 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Eric Hanchrow <eric.hanchrow <at> gmail.com>
To: emacs-pretest-bug <at> gnu.org
Subject: 23.0.90; PATCH: getenv mistakenly passes a frame to getenv_internal, 
	when it should instead pass a CONS
Date: Mon, 9 Feb 2009 19:37:47 -0800

Please write in English if possible, because the Emacs maintainers
usually do not have translators to read other languages for them.

Your bug report will be posted to the emacs-pretest-bug <at> gnu.org mailing list.

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

I noticed that (getenv "SSH_TTY" (selected-frame)) returned nil even
though (assoc 'environment (frame-parameters (selected-frame))) returned
an environment that indeed defined that variable.  I poked around and
saw that getenv was passing a frame to getenv_internal, but
getenv_internal only consults that second argument if it's a CONS.  This
patch seems to fix the problem.

    diff --git a/lisp/env.el b/lisp/env.el
    index 8d8bebe..ad88ba8 100644
    --- a/lisp/env.el
    +++ b/lisp/env.el
    @@ -198,7 +198,7 @@ in the environment list of the selected frame."
                                        (encode-coding-string
                                         variable locale-coding-system)
                                      variable)
    -				frame)))
    +				(and frame (assoc 'environment (frame-parameters frame))))))
         (if (and enable-multibyte-characters value)
            (setq value (decode-coding-string value locale-coding-system)))
         (when (interactive-p)


If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
If you would like to further debug the crash, please read the file
/usr/local/src/emacs/etc/DEBUG for instructions.


In GNU Emacs 23.0.90.1 (i686-pc-linux-gnu, GTK+ Version 2.14.4)
 of 2009-02-05 on debian
Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: nil
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: Emacs-Lisp

Minor modes in effect:
  auto-fill-function: do-auto-fill
  recentf-mode: t
  erc-ring-mode: t
  erc-pcomplete-mode: t
  erc-netsplit-mode: t
  eldoc-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-autojoin-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-services-mode: t
  erc-networks-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-readonly-mode: t
  desktop-save-mode: t
  display-time-mode: t
  global-auto-revert-mode: t
  diff-auto-refine-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
F RET C-x n d C-x b RET ESC C-n ESC C-p ESC C-n ESC
C-p ESC C-n ESC C-f C-x b RET C-u C-n C-u C-n C-u C-n
C-p C-p C-e ESC b C-s C-w C-s C-s C-a C-x b RET C-x
b RET ESC C-n ESC C-n C-n C-x n w ESC C-d C-s C-w C-w
C-s C-s C-s C-s C-s C-s C-a C-x n d C-x n w C-x 2 C-u
C-@ ESC - C-l C-l C-x 0 C-x n d C-e ESC b C-s C-w C-s
C-s C-s C-s C-s C-s C-s C-a C-x b RET C-x n w C-x v
g l ESC 0 C-l C-x k RET C-x o C-x b i n i RET C-x b
s c r RET C-p C-p ESC C-@ ESC w C-x o ESC m C-y ESC
C-k TAB ESC b ESC C-u ESC C-k f r a m e C-x o ESC >
ESC ( f r a m ESC / SPC n i l C-e C-j ESC C-p C-k C-p
ESC C-d C-h f RET C-x b RET C-l ESC m ESC 1 ESC ( a
n d SPC f r a m e SPC C-x C-s ESC C-x C-x b C-s RET
ESC C-u ESC C-n C-x C-e C-x e DEL C-x b e m a RET C-g
C-r s n o g ESC > r u d TAB s e e n SPC s n o g TAB
g ESC / RET n u t s . RET C-x b RET C-x o C-x b C-s
RET ESC x b y t e - r TAB RET RET RET y C-x v = C-x
o ESC x r e p o r t - e m a c s - b u g RET

Recent messages:
Checking /usr/local/src/emacs/lisp/obsolete...
Checking /usr/local/src/emacs/lisp/org...
Checking /usr/local/src/emacs/lisp/play...
Checking /usr/local/src/emacs/lisp/progmodes...
Checking /usr/local/src/emacs/lisp/term...
Checking /usr/local/src/emacs/lisp/textmodes...
Checking /usr/local/src/emacs/lisp/url...
Done (Total of 1 file compiled)
Finding changes in /usr/local/src/emacs/lisp/env.el...done
Scanning for dabbrevs...100%

-- 
If there were a little guy running around inside the computer
executing our programs, he would probably have as long and
plaintive a tale to tell about his job as a federal government
employee.
        -- Paul Graham

Message #12 received at 2259-done <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Eric Hanchrow <eric.hanchrow <at> gmail.com>,
        Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 2259-done <at> debbugs.gnu.org
Subject: Re: bug#2259: 23.0.90; PATCH: getenv mistakenly passes a frame to 
	getenv_internal, when it should instead pass a CONS
Date: Tue, 10 Feb 2009 11:24:42 +0100

On Tue, Feb 10, 2009 at 04:37, Eric Hanchrow <eric.hanchrow <at> gmail.com> wrote:

> I poked around and
> saw that getenv was passing a frame to getenv_internal, but
> getenv_internal only consults that second argument if it's a CONS.  This
> patch seems to fix the problem.

I think the patch is right, so I'm committing it.  getenv-internal's
docstring used to say that it searches in the frame environment, but
it wasn't right and was recently fixed. (The few) other uses of
getenv-internal rightly treat ENV as a list of environment variables.

Stefan, I just saw that `server-with-environment' does this:

         (let ((,value (getenv-internal ,var ,env)))
           (push (if (null ,value)
                     ,var
                   (concat ,var "=" ,value))
                 process-environment)))

which will fail if ENV contains a negative entry. Is that a bug, or it
is known at that point that the ENV arg of `server-with-environment'
will never contain negative entries (i.e., can the output of
(process-get process 'env) contain negative entries)? If it is
intended, perhaps a comment would be in order.

    Juanma

Message #17 received at 2259 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Juanma Barranquero <lekktu <at> gmail.com>
To: Eric Hanchrow <eric.hanchrow <at> gmail.com>,
        Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 2259 <at> debbugs.gnu.org
Subject: Re: bug#2259: 23.0.90; PATCH: getenv mistakenly passes a frame to 
	getenv_internal, when it should instead pass a CONS
Date: Tue, 10 Feb 2009 17:10:20 +0100

On Tue, Feb 10, 2009 at 11:24, Juanma Barranquero <lekktu <at> gmail.com> wrote:

> or it
> is known at that point that the ENV arg of `server-with-environment'
> will never contain negative entries (i.e., can the output of
> (process-get process 'env) contain negative entries)? If it is
> intended, perhaps a comment would be in order.

OK, I see now that "ENV should be in the same format as
`process-environment'", and that, in `process-environment', "[e]ach
element should be a string of the form ENVVARNAME=VALUE". So yes, it
is known that there shouldn't be negative entries, and it is even
documented.

Still, perhaps this change would be useful...

    Juanma


Index: server.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/server.el,v
retrieving revision 1.188
diff -u -2 -r1.188 server.el
--- server.el	7 Feb 2009 21:59:35 -0000	1.188
+++ server.el	10 Feb 2009 15:55:04 -0000
@@ -241,7 +241,7 @@
        (dolist (,var ,vars)
          (let ((,value (getenv-internal ,var ,env)))
-           (push (if (null ,value)
-                     ,var
-                   (concat ,var "=" ,value))
+           (push (if (stringp ,value)
+                     (concat ,var "=" ,value)
+                   ,var)
                  process-environment)))
        (progn ,@body))))

Message #22 received at 2259 <at> emacsbugs.donarmstrong.com (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juanma Barranquero <lekktu <at> gmail.com>
Cc: Eric Hanchrow <eric.hanchrow <at> gmail.com>, 2259 <at> debbugs.gnu.org
Subject: Re: bug#2259: 23.0.90; PATCH: getenv mistakenly passes a frame to
Date: Tue, 10 Feb 2009 20:45:31 -0500

> OK, I see now that "ENV should be in the same format as
> `process-environment'", and that, in `process-environment', "[e]ach
> element should be a string of the form ENVVARNAME=VALUE". So yes, it
> is known that there shouldn't be negative entries, and it is even
> documented.

> Still, perhaps this change would be useful...

Yes, it looks good,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.