.emacs.desktop file destroyed by custom function

Question

I use the Emacs desktop module to save my open buffers between sessions. However I found that this accumulates more buffers than I want, so I wrote a small function to clean up the buffer list immediately before saving to the desktop file. This works as expected, but for strange reasons the .emacs.desktop gets scrambled occasionally, i.e. it contains a part of another buffer at its start, then the intended contents and then the result of the other buffer. I don't have the slightest idea, why this happens. Here is an excerpt from my .emacs file:

(defun kill-old-buffers ()
  "Kill buffers from end of buffer list (not used recently) until no more than 50 buffers are left. Remove temporary buffers first"
  (interactive)
  (let* (desktop-buffer (current-buffer))
   (dolist (buffer (buffer-list))
     (if (or (string-match "^\*" (buffer-name buffer)) (string-match "\.hpp$" (buffer-name buffer)))
         (kill-buffer buffer)
       )
     )

  (setq all-buffers (reverse (buffer-list)))
  (while (> (safe-length all-buffers) 50)
    (setq buffer (pop all-buffers))
    (if (not (string-equal (buffer-name buffer) (buffer-name (current-buffer))))
        (kill-buffer buffer)
      )
    )
  (switch-to-buffer desktop-buffer)
))

;; Kill old rarely-used buffers before saving
(add-hook 'desktop-save-hook
  '(lambda () (kill-old-buffers)))

Any help would be appreciated.

Answer 1

I'm not sure if your function is really the cause of your problem. If it should be the case, the wrong usage of the let* that scottfrazer pointed out might be the cause. But you don't even need that let* (and switch-to-buffer) at all, because

1. what you're trying to do is better done with Emacs' built in save-excursion, and
2. you aren't actually switching the buffer ever.

OTOH, you should have used let instead of the setqs in the lower half of your function, because setq will otherwise change a variable from an enclosing lexical scope. In this case you might very well have stomped over a buffer variable from the function that's executing the desktop-save-hook which is another potential cause of your problem.

But you don't need those lets either because you can do the second loop with another dolist. You can get rid of those first 50 buffers that you don't want to loop over with nthcdr.

Here's my improved version of kill-old-buffers:

(defun kill-old-buffers ()
  "Kill buffers from end of buffer list (not used recently) until
no more than 50 buffers are left. Remove temporary buffers first."
  (interactive)
  (save-excursion
    (dolist (buffer (buffer-list))
      (if (or (string-match "^\*" (buffer-name buffer))
              (string-match "\.hpp$" (buffer-name buffer)))
          (kill-buffer buffer)))

    (dolist (buffer (reverse (nthcdr 50 (buffer-list))))
      (unless (eq buffer (current-buffer))
        (kill-buffer buffer)))))

Answer 2

It may not fix all your problems, but for starters you need another set of parens around the variable in your let* statement

(let* ((desktop-buffer (current-buffer)))