From: Leo Prikler <leo.prikler@student.tugraz.at>
To: Maxime Devos <maximedevos@telenet.be>, 47849@debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Subject: [bug#47849] [PATCH v3 3/3] services: Add a service for Jami.
Date: Sun, 01 Aug 2021 09:58:02 +0200 [thread overview]
Message-ID: <a2aa7761985c5a9b4c2f90a9292723bc79a0dd2a.camel@student.tugraz.at> (raw)
In-Reply-To: <20210801065857.27657-4-maxim.cournoyer@gmail.com>
Hi,
Am Sonntag, den 01.08.2021, 02:58 -0400 schrieb Maxim Cournoyer:
> [...]
> + (let* ((command `(,@(if dbus-send
> + (list dbus-send)
> + (list (%send-dbus-binary)))
You can use non-splicing comma notation for things that are never
supposed to be lists.
> + ,@(if (or bus (%send-dbus-bus))
> + (list (string-append "--bus="
> + (or bus (%send-dbus-
> bus))))
> + '())
You use this style here, but
> + (let* ((uid (or (and=> (or user (%send-dbus-user))
> + (compose passwd:uid getpwnam)) -1))
> + (gid (or (and=> (or group (%send-dbus-group))
> + (compose group:gid getgrnam)) -1)))
this style here. I personally think using and=> everywhere would make
things a little clearer.
More importantly, though
> +(define* (send-dbus #:key service path interface method
> + bus
> + dbus-send
> + user group
> + timeout
> + arguments)
If you e.g. write (bus (%send-dbus-bus)), I think the argument should
already be correctly passed upon the function call, no?
> + (chown temp-port uid gid)))
> + (lambda ()
> + (let ((pid (fork+exec-command command
> + #:user (or user (%send-dbus-
> user))
> + #:group (or group (%send-dbus-
> group))
> + #:log-file temp-file)))
> + (match (waitpid pid)
> + ((_ . status)
> + (let ((exit-status (status:exit-val status))
> + (output (call-with-port temp-port get-string-
> all)))
> + (if (= 0 exit-status)
> + output
> + (error "the send-dbus command exited with: "
> + command exit-status output)))))))
Since output is unused in the error case, I think you can move the get-
string-all there. It would make a difference if you were to e.g. close
temp-port before that.
I did not look at the rest of this patch, but 1+2 LGTM.
Regards,
next prev parent reply other threads:[~2021-08-01 7:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <handler.47849.B47849.162151424825211@debbugs.gnu.org>
2021-08-01 6:58 ` [bug#47849] [PATCH v3] services: Add a service for the Jami daemon Maxim Cournoyer
2021-08-01 6:58 ` [bug#47849] [PATCH v3 1/3] .dir-locals.el: Specify indentation rule for with-shepherd-action Maxim Cournoyer
2021-08-01 6:58 ` [bug#47849] [PATCH v3 2/3] build: shepherd: Use autoload to lazily bind Shepherd modules Maxim Cournoyer
2021-08-01 6:58 ` [bug#47849] [PATCH v3 3/3] services: Add a service for Jami Maxim Cournoyer
2021-08-01 7:58 ` Leo Prikler [this message]
2021-08-02 19:17 ` bug#47849: [PATCH] Add a jami-daemon service Maxim Cournoyer
2021-08-03 7:35 ` [bug#47849] " Leo Prikler
2021-08-03 14:36 ` Maxim Cournoyer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a2aa7761985c5a9b4c2f90a9292723bc79a0dd2a.camel@student.tugraz.at \
--to=leo.prikler@student.tugraz.at \
--cc=47849@debbugs.gnu.org \
--cc=maxim.cournoyer@gmail.com \
--cc=maximedevos@telenet.be \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).