defadvice file-expand-wildcards ad-do-it value

[Kevin Ryde]

Doing a bit of a grep I noticed the defadvice tramp puts on
file-expand-wildcards may use the value from ad-do-it.

I believe that value can't be relied on, that it should look at
ad-return-value since ad-do-it is only whatever the next innermost
advice evaluates to, which can be anything.

I had similar in some of my own code, and had a bit of trouble tracking
down what I'd done wrong :-).

Perhaps along the lines below, except untested as I'm not smart enough
to reach the affected code.

Incidentally, the ad-unadvise in tramp-unload-hook probably should
confine itself to the particular piece of advice added, so as not to
affect anyone else's advices on file-expand-wildcards (in the unlikely
event there are any others!).

--- tramp.el.~2.743.~   2009-10-15 23:12:56.000000000 +1100
+++ tramp.el    2009-10-22 08:08:40.000000000 +1100
@@ -8297,10 +8297,15 @@
          (if (string-match
               "[[*?]"
               (tramp-file-name-localname (tramp-dissect-file-name name)))
-             (setq ad-return-value (or ad-do-it (list name)))
+             (progn
+               ad-do-it
+               (unless ad-return-value
+                 (setq ad-return-value (list name))))
            (setq ad-return-value (list name)))
        ;; If it is not a Tramp file, just run the original function.
-       (setq ad-return-value (or ad-do-it (list name))))))
+       ad-do-it
+       (unless ad-return-value
+         (setq ad-return-value (list name))))))
   (add-hook 'tramp-unload-hook
            (lambda () (ad-unadvise 'file-expand-wildcards))))