GNU bug report logs - #47364
[PATCH 0/2] Add pam-gnupg and PAM rules for SLiM

Previous Next

Full log

Message #14 received at 47364 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Oleg Pykhalov <go.wigust <at> gmail.com>, 47364 <at> debbugs.gnu.org
Subject: Re: [bug#47364] [PATCH 2/2] services: slim: Add pam-gnupg support.
Date: Wed, 24 Mar 2021 20:22:43 +0100

[Message part 1 (text/plain, inline)]

Hi,

I'm not familiar with PAM, so I can't do much reviewing about that
(seems ok, though I'm no expert).  Some nitpicks:

On Wed, 2021-03-24 at 19:52 +0300, Oleg Pykhalov wrote:
> [...]
> 
> diff --git a/gnu/system/pam.scm b/gnu/system/pam.scm
> index ad02586be8..75edd01908 100644
> --- a/gnu/system/pam.scm
> +++ b/gnu/system/pam.scm
> [...]
>     (lambda* (name #:key allow-empty-passwords? (allow-root? #f) motd
> -                   login-uid?)
> +                   login-uid? (gnupg? #f))

Nitpick: keyword variables have #f as default by default, so you could just write ...

     (lambda* (name #:key allow-empty-passwords? (allow-root? #f) motd
> -                   login-uid?)
> +                   login-uid? gnupg?)

... here.  As a minimal example, you could run the following code in a Guile REPL:

> ;; These both evaluate to (#f #f)!
> ((lambda* (#:key login-uid? gnupg?) (list login-uid? gnupg?)))
> ((lambda* (#:key login-uid? (gnupg? #f)) (list login-uid? gnupg?)))

Hmm, maybe (allow-root? #f) could be replaced with simply allow-root? here ...

>        "Return a standard Unix-style PAM service for NAME.  When
>  ALLOW-EMPTY-PASSWORDS? is true, allow empty passwords.  When ALLOW-ROOT? is
>  true, allow root to run the command without authentication.  When MOTD is

It would be nice if this docstring documents GNUPG? as well.

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Previous Next

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