Control of fan-speed on Lenovo Thinkpads

Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

Here is how I control Lenovo Thinkpad T410 (and others) fan speed. I
remember controlling it on different notebooks, I just forgot
which. You may find that you need to change variable
`my-fan-proc-file' to something else. Most of people do not need this,
it will be automatically controlled. But I need it sometimes to cool
down my fan and my palms.

(defun sudo (arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list arguments))))
	 (return (shell-command-to-string command)))
    return))

(defvar my-fan-proc-file "/proc/acpi/ibm/fan")

(defun fan-speed (arg)
  "Controls fan speed provided user has superuser rights and system
command `sudo' works without password. Function will mostly work
on Lenovo Thinkpad notebooks. Prefix argument ARG shall be
numbers from 1 to 9. Numbers from 1 to 7 determine the fan speed
level, number 8 represents auto fan speed level and number 9
represents full fan speed. Activate with `C-u NUMBER M-x
fan-speed'. If invoked without arguments it will show the
contents of the fan control file."
  (interactive "P")
  (when (and (file-exists-p my-fan-proc-file)
	     (rcd-which "sudo"))
    (if arg
	(cond ((= arg 7) (sudo (format "echo level %s >> %s" arg my-fan-proc-file)))
	      ((= arg 8) (sudo (format "echo level auto >> %s" my-fan-proc-file)))
	      ((= arg 9) (sudo (format "echo level full-speed >> %s" my-fan-proc-file)))
	      ((and (< arg 7) (> arg 0)) (if (y-or-n-p (format "Really set fan speed to %s?" arg))
					     (sudo (format "echo level %s >> %s" 7 my-fan-proc-file))))
	      (t (message "Supply better numeric argument to this function from 1 to 9.")))
      (message (file-to-string my-fan-proc-file)))))

Thanks,
Jean Louis

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Control of fan-speed on Lenovo Thinkpads

[Michael Albinus]

Jean Louis <bugs@gnu.support> writes:

Hi,

> (defun sudo (arguments)
>   "Executes list ARGUMENTS with system command `sudo'."
>   (let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list arguments))))
> 	 (return (shell-command-to-string command)))
>     return))

This could be

(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((default-directory "/sudo::"))
    (shell-command-to-string (string-join arguments " "))))

> Thanks,
> Jean Louis

Best regards, Michael.

Re: Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

* Michael Albinus <michael.albinus@gmx.de> [2021-03-30 11:13]:
> Jean Louis <bugs@gnu.support> writes:
> 
> Hi,
> 
> > (defun sudo (arguments)
> >   "Executes list ARGUMENTS with system command `sudo'."
> >   (let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list arguments))))
> > 	 (return (shell-command-to-string command)))
> >     return))
> 
> This could be
> 
(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((default-directory "/sudo::"))
    (shell-command-to-string (string-join arguments " "))))

thank you. I will keep it for future considerations. I think it works
slower as it invokes tramp and many other functions in background. Is
there any practical benefit?

I have tried using that function with (sudo "ls") and first I got the
~/tmp directory listing, then in second invokation, I got only "bin"
listing, as it changed to default directory to be /root where I have
"bin" only.

My function would not change default-directory to /root -- maybe that
is result of using default-directory, and it remains so maybe for the
rest of tramp session. Try it out, it makes no sense to be changed.

Your function that involves Tramp processes:

(sudo "ls") → "admin_Leo
babel-6KitXT
babel-cyz28q
babel-YJWc0L
babel-ZlWVex"

this is because I invoked editing of email message file in ~/tmp

but on second invokation, without changing anything, few seconds thereafter:

(sudo "ls") → "bin
"

(sudo "pwd") → "/root
"

This could be a bug, I do not know why should default directory be
changed to "/root" if I am not root, but used /sudo::

(defun sudo (arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list arguments))))
	 (return (shell-command-to-string command)))
    return))

(sudo "ls") → " 0install-ea1f1e-download
 847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
 adb.1001.log
 ... and others in ~/tmp
 
 "
Repeated (sudo "ls") shows same thing, without tramp.

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

* Michael Albinus <michael.albinus@gmx.de> [2021-03-30 11:14]:
> This could be
> 
> (defun sudo (&rest arguments)
>   "Executes list ARGUMENTS with system command `sudo'."
>   (let ((default-directory "/sudo::"))
>     (shell-command-to-string (string-join arguments " "))))

(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((original-directory default-directory)
	(default-directory "/sudo::")
	(return (shell-command-to-string (string-join arguments " "))))
    (setq default-directory original-directory)
    return))

This way it works better.

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Control of fan-speed on Lenovo Thinkpads

[Michael Albinus]

Jean Louis <bugs@gnu.support> writes:

Hi Jean,

> (defun sudo (&rest arguments)
>   "Executes list ARGUMENTS with system command `sudo'."
>   (let ((original-directory default-directory)
> 	(default-directory "/sudo::")
> 	(return (shell-command-to-string (string-join arguments " "))))
>     (setq default-directory original-directory)
>     return))

There's no need to keep and set original-directory, this is done by the
let wrapper.

And setting default-directory doesn't do anything, you would need let* in
your approach.

With my adapted code snippet, you could do the following in order to
keep the current directory:

(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((default-directory
	  (concat "/sudo::" (file-local-name default-directory))))
    (shell-command-to-string (string-join arguments " "))))

Best regards, Michael.

Finding simpler better sudo for Emacs

[Jean Louis]

* Michael Albinus <michael.albinus@gmx.de> [2021-03-30 13:14]:
> Jean Louis <bugs@gnu.support> writes:
> 
> Hi Jean,
> 
> > (defun sudo (&rest arguments)
> >   "Executes list ARGUMENTS with system command `sudo'."
> >   (let ((original-directory default-directory)
> > 	(default-directory "/sudo::")
> > 	(return (shell-command-to-string (string-join arguments " "))))
> >     (setq default-directory original-directory)
> >     return))
> 
> There's no need to keep and set original-directory, this is done by the
> let wrapper.

Maybe I know what you mean, as let wrapper should not set globally the
default directory, but practically, I have shown you that it is
changing to /root so that may be something done by tramp in
background, I do not know.

> And setting default-directory doesn't do anything, you would need let* in
> your approach.

Well, the function above did not change the default directoy, I would
not know what you mean. Important in that past moment is that it did
work that way. 

> With my adapted code snippet, you could do the following in order to
> keep the current directory:
> 
(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((default-directory
  	  (concat "/sudo::" (file-local-name default-directory))))
    (shell-command-to-string (string-join arguments " "))))

(sudo "ls") → " 0install-ea1f1e-download
 847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
 adb.1001.log" ;; are files from my tmp

but then if I change to dired buffer ~/tmp and do: M-: -- (sudo "ls")
I get following error:

"sh: cd: /root/tmp/: No such file or directory

So, no, no, it does not work as you and me intended.

This one here still works better as it remembers the current
directory and brings it back, as Tramp probably does some change in
background, don't know why.

(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((original-directory default-directory)
	(default-directory "/sudo::")
	(return (shell-command-to-string (string-join arguments " "))))
    (setq default-directory original-directory)
    return))
 
-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Finding simpler better sudo for Emacs

[Michael Albinus]

Jean Louis <bugs@gnu.support> writes:

Hi Jean,

> (defun sudo (&rest arguments)
>   "Executes list ARGUMENTS with system command `sudo'."
>   (let ((default-directory
>   	  (concat "/sudo::" (file-local-name default-directory))))
>     (shell-command-to-string (string-join arguments " "))))
>
> (sudo "ls") → " 0install-ea1f1e-download
>  847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
>  adb.1001.log" ;; are files from my tmp
>
> but then if I change to dired buffer ~/tmp and do: M-: -- (sudo "ls")
> I get following error:
>
> "sh: cd: /root/tmp/: No such file or directory

OK, so let's do

(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((default-directory
  	  (concat "/sudo::" (file-local-name (expand-file-name default-directory)))))
    (shell-command-to-string (string-join arguments " "))))

Best regards, Michael.

Re: Finding simpler better sudo for Emacs

[Jean Louis]

* Michael Albinus <michael.albinus@gmx.de> [2021-03-30 13:35]:
> Jean Louis <bugs@gnu.support> writes:
> 
> Hi Jean,
> 
> > (defun sudo (&rest arguments)
> >   "Executes list ARGUMENTS with system command `sudo'."
> >   (let ((default-directory
> >   	  (concat "/sudo::" (file-local-name default-directory))))
> >     (shell-command-to-string (string-join arguments " "))))
> >
> > (sudo "ls") → " 0install-ea1f1e-download
> >  847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
> >  adb.1001.log" ;; are files from my tmp
> >
> > but then if I change to dired buffer ~/tmp and do: M-: -- (sudo "ls")
> > I get following error:
> >
> > "sh: cd: /root/tmp/: No such file or directory
> 
> OK, so let's do
> 
(defun sudo (&rest arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((default-directory
  	  (concat "/sudo::" (file-local-name (expand-file-name default-directory)))))
    (shell-command-to-string (string-join arguments " "))))

(sudo "ls") → " 0install-ea1f1e-download
 847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
 adb.1001.log
 ..."

(sudo "ls") → " 0install-ea1f1e-download
 847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
 adb.1001.log
 ..."

That one works well. I will keep it.

On the other hand, now I have got concern for this command:

(defun sudo (arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list arguments))))
	 (return (shell-command-to-string command)))
    return))

As I would not like, definitely I would not like executing my sudo
commands on remote servers which I use so often with the above sudo
command.

So I would like to make sure I am on the local file system, as if I am
on /sudo:: file system or /ssh: or similar, that is not something I
would like. I have multiple notebooks which I access, imagine I change
default directory to some remote one and I am not visible in the
buffer of Dired. This happens sometimes. Then I wish to execute fan
control, but it would run on remote computer.

How to check if default directory is on local file system?

 
-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Finding simpler better sudo for Emacs

[Michael Albinus]

Jean Louis <bugs@gnu.support> writes:

> How to check if default directory is on local file system?

(file-remote-p default-directory)

nil if you are on your local host, non-nil otherwise.

Best regards, Michael.

Re: Finding simpler better sudo for Emacs

[Jean Louis]

* Michael Albinus <michael.albinus@gmx.de> [2021-03-30 13:53]:
> Jean Louis <bugs@gnu.support> writes:
> 
> > How to check if default directory is on local file system?
> 
> (file-remote-p default-directory)
> 
> nil if you are on your local host, non-nil otherwise.

Thank you, this time I was M-x apropos-ing but did not find it so
fast.

Side note on tramp, I have filed a bug on Dired/Tramp as if there is
local directory /scp:static: and I try to enter that directory I am
connected with Tramp to my remote static (hostname defined in
/etc/hosts or ~/.authinfo) server. But Dired should recognize that
marked file was on the local directory and should not go into remote
one.

In that sense if there is local directory "/scp:static:" then this
function does not work well:

(file-remote-p "/scp:static:") → "/scp:static:" while you said it
should give nil if it is local directory.

(file-remote-p "/scp:google.com:") → "/scp:google.com:" -- so this
seem that it is checking it by prefix.

That means that users are not supposed to make such local directories,
for example making a local directory /sudo: or /scp:hostname: and
similar would make Tramp fail. Emacs and Tramp rely on improbability
that users would never make such local directories. Copy and rename of
files into such local directories would probably fail, as Tramp would
try to copy it remotely.

Regarding my function, I like it to be executed only locally, and I
will skip Tramp usage for a while.

(defun sudo (arguments)
  "Executes list ARGUMENTS with system command `sudo'."
  (let ((not-remote (not (file-remote-p default-directory))))
    (if not-remote
	(let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list arguments))))
	       (return (shell-command-to-string command)))
	  return)
      (message "This `sudo' does not work on remote directory: %s" default-directory))))

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Finding simpler better sudo for Emacs

[Michael Albinus]

Jean Louis <bugs@gnu.support> writes:

Hi Jean,

> Side note on tramp, I have filed a bug on Dired/Tramp as if there is
> local directory /scp:static: and I try to enter that directory I am
> connected with Tramp to my remote static (hostname defined in
> /etc/hosts or ~/.authinfo) server. But Dired should recognize that
> marked file was on the local directory and should not go into remote
> one.

I've closed this, because Tramp handles this case. See my other message.

Best regards, Michael.

Re: Finding simpler better sudo for Emacs

[Jean Louis]

* Michael Albinus <michael.albinus@gmx.de> [2021-03-30 14:14]:
> Jean Louis <bugs@gnu.support> writes:
> 
> Hi Jean,
> 
> > Side note on tramp, I have filed a bug on Dired/Tramp as if there is
> > local directory /scp:static: and I try to enter that directory I am
> > connected with Tramp to my remote static (hostname defined in
> > /etc/hosts or ~/.authinfo) server. But Dired should recognize that
> > marked file was on the local directory and should not go into remote
> > one.
> 
> I've closed this, because Tramp handles this case. See my other message.

I see now, there is variable to exclude some of /root-based
directories. Unlikely, but useful.


-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Control of fan-speed on Lenovo Thinkpads

[Stefan Monnier]

> (defun sudo (&rest arguments)
>   "Executes list ARGUMENTS with system command `sudo'."
>   (let ((default-directory "/sudo::"))
>     (shell-command-to-string (string-join arguments " "))))

Please don't `string-join` arguments and please don't use a shell when
it's not necessary: they're recipes for quoting bugs.
Better use something like `file-process` here.


        Stefan

Re: Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

* Stefan Monnier <monnier@iro.umontreal.ca> [2021-03-30 18:03]:
> > (defun sudo (&rest arguments)
> >   "Executes list ARGUMENTS with system command `sudo'."
> >   (let ((default-directory "/sudo::"))
> >     (shell-command-to-string (string-join arguments " "))))
> 
> Please don't `string-join` arguments and please don't use a shell when
> it's not necessary: they're recipes for quoting bugs.
> Better use something like `file-process` here.

Yes, you are right, that it works well is coincidence. I think that
string-join came from earlier command which used call-process or
similar where arguments are separate.

I have improved my command, and it still works well. 

(defun sudo (command)
  "Execute COMMAND with system command `sudo'."
  (let ((not-remote (not (file-remote-p default-directory))))
    (if not-remote
	(let* ((command (format "sudo su -c -- root -c \"%s\"" command))
	       (return (shell-command-to-string command)))
	  return)
      (message "This `sudo' does not work on remote directory: %s" default-directory))))

You probably mean process-file? There is no file-process.

There is practical burden to use call-process, and it is with &rest
ARGS, as those have to be given all separately.

One echo to file and command with sudo becomes very complex, and it does not work:

(call-process "/usr/bin/sudo" nil (get-buffer-create "sudo") nil "su" "-c" "--" "root" "-c" "echo" "level" "full-speed" ">" "/proc/acpi/ibm/fan")

(call-process "/usr/bin/sudo" nil (get-buffer-create "sudo") nil "su" "-c" "--" "root" "-c" "echo" "hello" ">" "/tmp/text.txt") → 0 -- but does not produce /tmp/text.txt, so I do not know how to use it.

Then again I would like to pass a command, and not quote each part of command as arguments.

I would like something like (sudo "echo hello > /proc/file") that it works.

But for that to work, how I understand, I need to split string with spaces, remove empty strings maybe, and then pass it as list as ARGS to call-process.

(setq args '("su" "-c" "--" "root" "-c" "echo" "level" "full-speed" ">" "/proc/acpi/ibm/fan"))

(call-process "sudo" nil t t args) -> this will not work, so to
pass it as a list there, I would need to make a macro expansion?

`(call-process "sudo" nil (get-buffer-create "sudo") nil ,@args) → (call-process "sudo" nil (get-buffer-create "sudo") nil "su" "-c" "--" "root" "-c" "echo" "level" "full-speed" ">" "/proc/acpi/ibm/fan")

(defun sudo (command)
  "Execute COMMAND with system command `sudo'."
  (let ((not-remote (not (file-remote-p default-directory))))
    (if not-remote
	(let* ((sudo "sudo")
	       (args (append '("su" "-c" "--" "root" "-c") (split-string command)))
	       (status `(call-process ,sudo nil (get-buffer-create "*sudo*") t ,@args)))
	  (message "%s" status)
	  (eval status)))))

By using that above function, this will work: (sudo "ls") but this will not work:
(sudo "ls /boot /var") -- so I do not know how to expand the list properly in the function. 

I would like to get following to work in simpler way by providing string:

(sudo "echo level full-speed > /proc/acpi/ibm/fan")

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Control of fan-speed on Lenovo Thinkpads

[Stefan Monnier]

> You probably mean process-file? There is no file-process.

Indeed, sorry.

> There is practical burden to use call-process, and it is with &rest
> ARGS, as those have to be given all separately.

There is no such extra burden.  Arguments always have to be given
separately.  If you forget about that, you get bugs and associated
security holes.  Executing a process is not the same as executing an
`shell` command.  The first is a fairly simple and safe operation
(assuming you trust the executable you're launching), whereas the second
is tricky and risky unless you're extra careful to quote everything just
right and you're sure the shell is really the one you expect, etc...

And if you really indeed get a shell command as input instead of an
executable along with its list of arguments, then it's trivial to turn
from `command` to `&rest args` by simply doing (list shell-file-name
shell-command-switch command).


        Stefan

Re: Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

* Stefan Monnier <monnier@iro.umontreal.ca> [2021-03-31 04:24]:
> > You probably mean process-file? There is no file-process.
> 
> Indeed, sorry.
> 
> > There is practical burden to use call-process, and it is with &rest
> > ARGS, as those have to be given all separately.
> 
> There is no such extra burden.  Arguments always have to be given
> separately.  If you forget about that, you get bugs and associated
> security holes.  Executing a process is not the same as executing an
> `shell` command.  The first is a fairly simple and safe operation
> (assuming you trust the executable you're launching), whereas the second
> is tricky and risky unless you're extra careful to quote everything just
> right and you're sure the shell is really the one you expect, etc...

Yes, and I follow the advise.

This works well:

(shell-command "sudo su -c -- root -c \"echo level full-speed > /proc/acpi/ibm/fan\"")

While the argument after last "-c" has to be full line, which again
defeats the principle of being extra carefull and quoting everything
in this "sudo" example. Other commands are fine. Following line works.

(call-process "sudo" nil (get-buffer-create "*sudo*") t "su" "-c" "--" "root" "-c" "echo level auto > /proc/acpi/ibm/fan")

Thus based on that line, it become clear that "sudo" command that
shall be able to redirect output must be made in such way to defeat
the purpose of quoting and being careful.

Thus the following line works with the function below without
problems. I have now implemented your advise but probably not in the
spirit how it was meant. but I do not know how to add the command to
below call-process line without making macro and using eval.

(sudo "echo level full-speed > /proc/acpi/ibm/fan")

(defun sudo (command)
  "Execute COMMAND with system command `sudo'."
  (let ((not-remote (not (file-remote-p default-directory)))
	(sudo-buffer (get-buffer-create "*sudo*"))
	(current-buffer (current-buffer)))
    (switch-to-buffer sudo-buffer)
    (erase-buffer)
    (switch-to-buffer current-buffer)
    (if not-remote
	(let* ((sudo `(call-process "sudo" nil ,sudo-buffer t "su" "-c" "--" "root" "-c" ,command))
	       (status (eval sudo))
	       (status (if (= 0 status) "Success" status))
	       (current-buffer (current-buffer))
	       (output (progn
			 (switch-to-buffer sudo-buffer)
			 (buffer-string))))
	  (switch-to-buffer current-buffer)
	  (message "%sStatus: %s" output status))
      (message "This `sudo' does not work on remote directory: %s" default-directory))))


-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Re: Control of fan-speed on Lenovo Thinkpads

[Stefan Monnier]

> 	(let* ((sudo `(call-process "sudo" nil ,sudo-buffer t "su" "-c" "--" "root" "-c" ,command))
> 	       (status (eval sudo))

Why?  No kitten needs to suffer here:

    (let* ((status (call-process "sudo" nil sudo-buffer t "su" "-c" "--" "root" "-c" command))


-- Stefan

Re: Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

* Stefan Monnier <monnier@iro.umontreal.ca> [2021-03-31 17:24]:
> > 	(let* ((sudo `(call-process "sudo" nil ,sudo-buffer t "su" "-c" "--" "root" "-c" ,command))
> > 	       (status (eval sudo))
> 
> Why?  No kitten needs to suffer here:
> 
>     (let* ((status (call-process "sudo" nil sudo-buffer t "su" "-c" "--" "root" "-c" command))

In that specific case yes. It works, I changed it. But it defeats
itself in purpose, do you see? That COMMAND is parameter to `su' which
invokes default user's shell. I don't find it bad, it just defeats the
purpose of `call-process'.

On the other hand, something like this would not work:

(call-process "dmesg" nil (get-buffer-create "*out*") t "-l info") → 1 with message "dmesg: unknown level ' info'"

but this works:

(call-process "dmesg" nil (get-buffer-create "*out*") t "-l" "info") → 0

Maybe `call-process' is not that much safer than `shell-command',
as with some dangerous arguments it could execute wrongly. I find
it beneficial that one can forget about quoting.

(call-process "echo" nil (get-buffer-create "*out*") t "-n" "Hello
there") → 0 -- works well even with new line.

(call-process "echo" nil (get-buffer-create "*out*") t "-n" "Hello \"there\" 'Somebody here' and ''' more than that.") → 0 with message: Hello "there" 'Somebody here' and ''' more than that.

For me it brings main benefit that I can forget about problems of
quoting. I will be replacing `shell-command' in those functions
where quoting is possible problem.

Example would be here, where `figlet' accepts strings but then
with `shell-command' I would need to take care of proper quoting.

(defvar figlet-history nil)
(defvar figlet-font-history nil)

(defun figlet ()
  (interactive)
  (let* ((fonts '("banner" "big" "block" "bubble" "digital"
		  "ivrit" "lean" "mini" "mnemonic" "script"
		  "shadow" "slant" "small" "smscript" "smshadow"
		  "smslant" "standard" "term"))
	 (font (completing-read "Font: " fonts nil t (car figlet-font-history) 'figlet-font-history))
	 (text (read-from-minibuffer "Text: " (car figlet-history) nil nil 'figlet-history))
	 (command (format "figlet -f %s \"%s\"" font text))
	 (figlet (shell-command-to-string command)))
    (if buffer-read-only
	(message figlet)
      (insert figlet))))

Then such can be improved so that programmer does not think of quoting:

(defun figlet ()
  (interactive)
  (let* ((fonts '("banner" "big" "block" "bubble" "digital"
		  "ivrit" "lean" "mini" "mnemonic" "script"
		  "shadow" "slant" "small" "smscript" "smshadow"
		  "smslant" "standard" "term"))
	 (font (completing-read "Font: " fonts nil t (car figlet-font-history) 'figlet-font-history))
	 (text (read-from-minibuffer "Text: " (car figlet-history) nil nil 'figlet-history)))
    (call-process "figlet" nil t nil "-f" font text)))

Where one can insert any kind of quotes in the string:

+-+-+-+-+-+-+-+ +-+-+-+-+-+
|"|t|h|a|n|k|"| |"|y|o|u|"|
+-+-+-+-+-+-+-+ +-+-+-+-+-+

or

+-+-+-+-+-+-+ +-+-+ +-+-+-+-+-+-+-+-+ +-+-+ +-+-+ +-+-+-+-+-+ +-+-+-+ +-+
|'|'|T|h|i|s| |i|s| |'|'|F|u|n|n|y|'| |\|'| |i|t| |w|o|r|k|s| |a|l|l| |-|
+-+-+-+-+-+-+ +-+-+ +-+-+-+-+-+-+-+-+ +-+-+ +-+-+ +-+-+-+-+-+ +-+-+-+ +-+
+-+-+-+-+-+-+-+-+-+-+
|||@|\|"|n|i|c|e|\|"|
+-+-+-+-+-+-+-+-+-+-+


-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Sign an open letter in support of Richard M. Stallman
https://rms-support-letter.github.io/

Re: Control of fan-speed on Lenovo Thinkpads

[Stefan Monnier]

> * Stefan Monnier <monnier@iro.umontreal.ca> [2021-03-31 17:24]:
>> > 	(let* ((sudo `(call-process "sudo" nil ,sudo-buffer t "su" "-c" "--" "root" "-c" ,command))
>> > 	       (status (eval sudo))
>> 
>> Why?  No kitten needs to suffer here:
>> 
>>     (let* ((status (call-process "sudo" nil sudo-buffer t "su" "-c" "--" "root" "-c" command))
>
> In that specific case yes. It works, I changed it. But it defeats
> itself in purpose, do you see? That COMMAND is parameter to `su' which
> invokes default user's shell. I don't find it bad, it just defeats the
> purpose of `call-process'.

You still have the risk of "command" being incorrect because of
messed up quoting, but not because of the shell you yourself invoke: just
because of the shell invoked by `su`.  And since "command" is what you
receive you can't do much better than that anyway.


        Stefan

Re: Control of fan-speed on Lenovo Thinkpads

[Jean Louis]

* Stefan Monnier <monnier@iro.umontreal.ca> [2021-03-31 23:20]:
> > * Stefan Monnier <monnier@iro.umontreal.ca> [2021-03-31 17:24]:
> >> > 	(let* ((sudo `(call-process "sudo" nil ,sudo-buffer t "su" "-c" "--" "root" "-c" ,command))
> >> > 	       (status (eval sudo))
> >> 
> >> Why?  No kitten needs to suffer here:
> >> 
> >>     (let* ((status (call-process "sudo" nil sudo-buffer t "su" "-c" "--" "root" "-c" command))
> >
> > In that specific case yes. It works, I changed it. But it defeats
> > itself in purpose, do you see? That COMMAND is parameter to `su' which
> > invokes default user's shell. I don't find it bad, it just defeats the
> > purpose of `call-process'.
> 
> You still have the risk of "command" being incorrect because of
> messed up quoting, but not because of the shell you yourself invoke: just
> because of the shell invoked by `su`.  And since "command" is what you
> receive you can't do much better than that anyway.

Thank you.

The lesson is more important than this particular `sudo' function. As
now I have to improve other 158 functions with `shell-command' for
safety of quoting.

-- 
Jean

Take action in Free Software Foundation campaigns:
https://www.fsf.org/campaigns

Sign an open letter in support of Richard M. Stallman
https://rms-support-letter.github.io/

Re: Control of fan-speed on Lenovo Thinkpads

[Utkarsh Singh]

Jean Louis <bugs@gnu.support> writes:

> * Utkarsh Singh <utkarsh190601@gmail.com> [2021-03-30 17:28]:
>> Hi Jean,
>> 
>> > (defun sudo (arguments)
>> >   "Executes list ARGUMENTS with system command `sudo'."
>> >   (let* ((command (format "sudo su -c -- root -c \"%s\"" (string-join (list 
>> > arguments))))
>> >          (return (shell-command-to-string command)))
>> >     return))
>> 
>> Why are you using a separate function for using sudo?
>
> That way it is distinguished.
>
>> Why not just have wrapper around call-process function?
>> 
>> Here is my approach (adapted from https://protesilaos.com/dotemacs/):
>> 
> (defun ut-common-shell-command (command &rest args)
>   "Run COMMAND with ARGS.
> Return the exit code and output in a list."
>   (with-temp-buffer
>     (list (apply 'call-process command nil (current-buffer) nil args)
>           (buffer-string))))
>
> (ut-common-shell-command "sudo su -c -- root -c \"echo level full-speed > /proc/acpi/ibm/fan\"")
>
> apply: Searching for program: No such file or directory, sudo su -c --
> root -c "echo level full-speed > /proc/acpi/ibm/fan" [2 times]
>
> Well not working how I would like it, I need to dissect it in each
> separate comand:
>
> (ut-common-shell-command "sudo" "su" "-c" "--" "root" "-c" "ls") → (0 "0install-ea1f1e-download
> 847cf980479cf1dd15482464efa8620ca98c1c93c5a3ac1e2a1dd24918b9cd46.html
> adb.1001.log
> ")
You can use something like this if you don't want to separate thing manually:
(let* ((cmd "ls -l -a")
       (cmd-list (split-string cmd " ")))
  (apply 'ut-common-shell-command cmd-list))

>
> and I am getting lists as output.
>
> This does not work as it should:
>
> (ut-common-shell-command "sudo" "su" "-c" "--" "root" "-c" "echo level full-speed > /proc/acpi/ibm/fan\")
>
> So you see the problem?
>
> This works with my function:
>
> (sudo (format "echo level %s > %s" arg my-fan-proc-file)))
>
>> I use this by giving "sudo" as first argument.  Here is a snippet from
>> the library I have written to mount and umount USB and Android devices
>> from emacs.
>> 
>> (defun emount-umount-usb (mount-point)
>>   "Umount usb mounted at MOUNT-POINT."
>>   (interactive (list (emount--select-umount-usb (emount--get-drive-list emount--list-drive))))
>>   (let* ((result (ut-common-shell-command "sudo" "umount" mount-point))
>> 	 (exit-code (nth 0 result))
>> 	 (output (nth 1 result)))
>>     (if (eq exit-code emount--success-code)
>> 	(message "%s umount successful" mount-point)
>>       (user-error output))))
>
> That is good.
>
> You see in this case with redirection of output, it is not so easy to
> provide a command with call-process.
>
Hmm I see the problem with redirection.  Do you know any other standard
Emacs way to work with redirection? Maybe temp buffer?
-- 
Utkarsh Singh
utkarshsingh.xyz