operating-system definitions allow duplicate passwd and group entries

  • Done
  • quality assurance status badge
Details
4 participants
  • Jason Conroy
  • Danny Milosavljevic
  • Leo Prikler
  • Ludovic Courtès
Owner
unassigned
Submitted by
Jason Conroy
Severity
normal
J
J
Jason Conroy wrote on 31 Dec 2020 19:14
(address . bug-guix@gnu.org)
CABWzUjVOFSgBUw-Wyx-+BWL2VWhfJR=CGutVZZP8ri2KrdtUnw@mail.gmail.com
When an operating-system contains multiple users or groups with the same
name, instantiating it with `guix system` does not cause a validation
failure, nor are the duplicate entries filtered from the resulting /etc
files.

This duplication can happen in a few different ways:

- both entries are manually included in the "users" or "groups" fields of
the operating-system
- a manually-specified entry collides with an entry defined by a service
(via an account-service-type extension)
- multiple services define entries that collide with each other

Steps to reproduce: call "guix system container" with the attached
operating-system definition.
Attachment: file
L
L
Leo Prikler wrote on 1 Jan 2021 12:13
[PATCH] system: Assert, that user and group names are unique.
(address . 45570@debbugs.gnu.org)(address . conjaroy@gmail.com)
20210101111309.7701-1-leo.prikler@student.tugraz.at
*gnu/system/shadow.scm (assert-unique-account-names)
(assert-unique-group-names): New variables.
(account-activation): Use them here.
---
gnu/system/shadow.scm | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

Toggle diff (48 lines)
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..61562f225e 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -222,6 +222,32 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
(rename-file ".nanorc" ".config/nano/nanorc"))
#t))))
+(define (assert-unique-account-names users)
+ (let loop ((names '())
+ (users users))
+ (unless (null? users)
+ (let ((name (user-account-name (car users))))
+ (if (member name names)
+ (raise (condition
+ (&message
+ (message
+ (format #f (G_ "account with name '~a' found twice")
+ name)))))
+ (loop (cons name names) (cdr users)))))))
+
+(define (assert-unique-group-names groups)
+ (let loop ((names '())
+ (groups groups))
+ (unless (null? groups)
+ (let ((name (user-account-name (car groups))))
+ (if (member name names)
+ (raise (condition
+ (&message
+ (message
+ (format #f (G_ "group with name '~a' found twice")
+ name)))))
+ (loop (cons name names) (cdr groups)))))))
+
(define (assert-valid-users/groups users groups)
"Raise an error if USERS refer to groups not listed in GROUPS."
(let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +318,8 @@ group."
(define group-specs
(map user-group->gexp groups))
+ (assert-unique-account-names accounts)
+ (assert-unique-group-names groups)
(assert-valid-users/groups accounts groups)
;; Add users and user groups.
--
2.29.2
D
D
Danny Milosavljevic wrote on 2 Jan 2021 02:16
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
20210102021601.74c2205a@scratchpost.org
Hi Leo,

I agree that this is a good idea.

Please use (ice-9 match) instead of car and cdr.

Something among these lines would be more transparent:

(define (find-duplicates list accessor)
(match list
(() '())
((head . tail)
(if (member head tail accessor) ; (srfi srfi-1) member
(cons head (find-duplicates tail accessor))
(find-duplicates tail accessor)))))

(find-duplicates users
(lambda (a b)
(string=? (user-account-name a)
(user-account-name b)))

(I think one could also use srfi-1 delete-duplicates and then compare the
lengths. Then the entire thing is a one-liner--the only complication is
to find the duplicates again after doing it (for the error message))
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCgAdFiEEds7GsXJ0tGXALbPZ5xo1VCwwuqUFAl/vyVEACgkQ5xo1VCww
uqWekAgAp5r/QdsNKZ96v1UhGjbotHLNOS0BfMsvDZafTK/h8FToyel/E7wOrKca
0NjR+C01HkhhYYGl5/WoLLpR103fSo/NXtH0TykyBe7D4WwUn46gE2x7D59G2qmW
f7ztIrkZke/JWEjQrSzsJwEY5uYT6D7b9dtYeUj7zBPU0Z+J+6+2nQYjZNLPrkDG
7GCZS8aJwmVBza6YKq/dmCM/nql8yV4kBpSXXEffhV4KEQAWdb51aS14wBIlEfj7
0y3Z64c0Xlgfd/6N5YSHlirn9gpNcRkO7jYiPwryweUK6LQRcWoeMXqM4oGJbTlU
V0gqbnNxQqWwQIASBIUN/FXW7vpIoQ==
=yiKX
-----END PGP SIGNATURE-----


L
L
Leo Prikler wrote on 2 Jan 2021 06:57
[PATCH] system: Assert, that user and group names are unique.
(address . 45570@debbugs.gnu.org)
20210102055728.22594-1-leo.prikler@student.tugraz.at
*gnu/system/shadow.scm (find-duplicates): New variable.
(assert-unique-account-names, assert-unique-group-names): New variables.
(account-activation): Use them here.
---
gnu/system/shadow.scm | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)

Toggle diff (62 lines)
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..3a5ea4dc70 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -34,6 +34,7 @@
#:use-module ((gnu packages admin)
#:select (shadow))
#:use-module (gnu packages bash)
+ #:use-module (ice-9 match)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
@@ -222,6 +223,38 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
(rename-file ".nanorc" ".config/nano/nanorc"))
#t))))
+(define (find-duplicates list =)
+ (match list
+ ('() '())
+ ((first . rest)
+ (if (member first rest =) ; (srfi srfi-1) member
+ (cons first (find-duplicates rest =))
+ (find-duplicates rest =)))))
+
+(define (assert-unique-account-names users)
+ (for-each
+ (lambda (account)
+ (raise (condition
+ (&message
+ (message
+ (format #f (G_ "account with name '~a' found twice.")
+ (user-account-name account)))))))
+ (find-duplicates users (lambda (alice bob)
+ (string=? (user-account-name alice)
+ (user-account-name bob))))))
+
+(define (assert-unique-group-names groups)
+ (for-each
+ (lambda (group)
+ (raise (condition
+ (&message
+ (message
+ (format #f (G_ "group with name '~a' found twice.")
+ (user-group-name group)))))))
+ (find-duplicates groups (lambda (red blue)
+ (string=? (user-group-name red)
+ (user-group-name blue))))))
+
(define (assert-valid-users/groups users groups)
"Raise an error if USERS refer to groups not listed in GROUPS."
(let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +325,8 @@ group."
(define group-specs
(map user-group->gexp groups))
+ (assert-unique-account-names accounts)
+ (assert-unique-group-names groups)
(assert-valid-users/groups accounts groups)
;; Add users and user groups.
--
2.29.2
L
L
Ludovic Courtès wrote on 6 Jan 2021 10:56
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87v9cao0c8.fsf@gnu.org
Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

Toggle quote (4 lines)
> *gnu/system/shadow.scm (find-duplicates): New variable.
> (assert-unique-account-names, assert-unique-group-names): New variables.
> (account-activation): Use them here.

[...]

Toggle quote (4 lines)
> +(define (find-duplicates list =)
> + (match list
> + ('() '())

This should be:

(match list
(() '())
…)

I’m surprised '() works as a pattern.

Toggle quote (5 lines)
> + ((first . rest)
> + (if (member first rest =) ; (srfi srfi-1) member
> + (cons first (find-duplicates rest =))
> + (find-duplicates rest =)))))

Note that this is quadratic; it’s fine as long as we don’t have “too
many” users, which may be the case in general.

Toggle quote (12 lines)
> +(define (assert-unique-account-names users)
> + (for-each
> + (lambda (account)
> + (raise (condition
> + (&message
> + (message
> + (format #f (G_ "account with name '~a' found twice.")
> + (user-account-name account)))))))
> + (find-duplicates users (lambda (alice bob)
> + (string=? (user-account-name alice)
> + (user-account-name bob))))))

‘for-each’ looks awkward since we’ll stop on the first one. How about
something like:

(define (assert-unique-account-names users)
(match (find-duplicates things …)
(() #t)
(lst
(raise (formatted-message (G_ "the following accounts appear more than once:~{ ~a~}~%"
lst))))))

?

Thanks!

Ludo’.
L
L
Leo Prikler wrote on 6 Jan 2021 13:34
(name . Ludovic Courtès)(address . ludo@gnu.org)
e1f7d49f48db237ece4527a8ee5236a8bfa629e7.camel@student.tugraz.at
Hi,

Am Mittwoch, den 06.01.2021, 10:56 +0100 schrieb Ludovic Courtès:
Toggle quote (22 lines)
> Hi,
>
> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>
> > *gnu/system/shadow.scm (find-duplicates): New variable.
> > (assert-unique-account-names, assert-unique-group-names): New
> > variables.
> > (account-activation): Use them here.
>
> [...]
>
> > +(define (find-duplicates list =)
> > + (match list
> > + ('() '())
>
> This should be:
>
> (match list
> (() '())
> …)
>
> I’m surprised '() works as a pattern.
I think it's because matching literals works, but you're right.

Toggle quote (7 lines)
> > + ((first . rest)
> > + (if (member first rest =) ; (srfi srfi-1) member
> > + (cons first (find-duplicates rest =))
> > + (find-duplicates rest =)))))
>
> Note that this is quadratic; it’s fine as long as we don’t have “too
> many” users, which may be the case in general.
It is indeed quadratic, but would there even be an n log n solution?
I've once done an n log n sort+delete-duplicates!, perhaps that'd be a
nicer solution here?

Toggle quote (26 lines)
> > +(define (assert-unique-account-names users)
> > + (for-each
> > + (lambda (account)
> > + (raise (condition
> > + (&message
> > + (message
> > + (format #f (G_ "account with name '~a' found
> > twice.")
> > + (user-account-name account)))))))
> > + (find-duplicates users (lambda (alice bob)
> > + (string=? (user-account-name alice)
> > + (user-account-name bob))))))
>
> ‘for-each’ looks awkward since we’ll stop on the first one. How
> about
> something like:
>
> (define (assert-unique-account-names users)
> (match (find-duplicates things …)
> (() #t)
> (lst
> (raise (formatted-message (G_ "the following accounts appear
> more than once:~{ ~a~}~%"
> lst))))))
>
> ?
That'd be weird for duplicate duplicates, hence just reporting the
first. Of course we could always count occurrences by allocating a
local hash table and then do some fancy hash-map->list conversion. If
we do use hash-tables, perhaps this could even be a linear algorithm?

Regards,
Leo
L
L
Ludovic Courtès wrote on 6 Jan 2021 14:32
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87k0sqkx7p.fsf@gnu.org
Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

Toggle quote (11 lines)
>> > + ((first . rest)
>> > + (if (member first rest =) ; (srfi srfi-1) member
>> > + (cons first (find-duplicates rest =))
>> > + (find-duplicates rest =)))))
>>
>> Note that this is quadratic; it’s fine as long as we don’t have “too
>> many” users, which may be the case in general.
> It is indeed quadratic, but would there even be an n log n solution?
> I've once done an n log n sort+delete-duplicates!, perhaps that'd be a
> nicer solution here?

You could first build a hash table or vhash or set with all the names,
then traverse again the list of names and check whether they’re in that
table. That’d be linear (assuming the table is well balanced), but the
constant factor would be higher.

Toggle quote (12 lines)
>> (define (assert-unique-account-names users)
>> (match (find-duplicates things …)
>> (() #t)
>> (lst
>> (raise (formatted-message (G_ "the following accounts appear
>> more than once:~{ ~a~}~%"
>> lst))))))
>>
>> ?
> That'd be weird for duplicate duplicates, hence just reporting the
> first.

You could do (delete-duplicates lst) in the message above?

Thanks,
Ludo’.
L
L
Leo Prikler wrote on 6 Jan 2021 22:00
(name . Ludovic Courtès)(address . ludo@gnu.org)
ae66ed3a5c4b862c9cc9ae2bdb6954a51434d79c.camel@student.tugraz.at
Hi,

Am Mittwoch, den 06.01.2021, 14:32 +0100 schrieb Ludovic Courtès:
Toggle quote (25 lines)
> Hi,
>
> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>
> > > > + ((first . rest)
> > > > + (if (member first rest =) ; (srfi srfi-1) member
> > > > + (cons first (find-duplicates rest =))
> > > > + (find-duplicates rest =)))))
> > >
> > > Note that this is quadratic; it’s fine as long as we don’t have
> > > “too
> > > many” users, which may be the case in general.
> > It is indeed quadratic, but would there even be an n log n
> > solution?
> > I've once done an n log n sort+delete-duplicates!, perhaps that'd
> > be a
> > nicer solution here?
>
> You could first build a hash table or vhash or set with all the
> names,
> then traverse again the list of names and check whether they’re in
> that
> table. That’d be linear (assuming the table is well balanced), but
> the
> constant factor would be higher.
Yeah, I think the hash table solution would make the most sense here.
Since VHashes are based on VLists, they're not actually purely
functional, are they?

Toggle quote (14 lines)
> > > (define (assert-unique-account-names users)
> > > (match (find-duplicates things …)
> > > (() #t)
> > > (lst
> > > (raise (formatted-message (G_ "the following accounts
> > > appear
> > > more than once:~{ ~a~}~%"
> > > lst))))))
> > >
> > > ?
> > That'd be weird for duplicate duplicates, hence just reporting the
> > first.
>
> You could do (delete-duplicates lst) in the message above?
Sure, but that'd be O(n^2) on top of O(n^2), which is less than ideal.
I think I'll try working on a hash-based implementation for now.

Regards,
Leo
L
L
Leo Prikler wrote on 6 Jan 2021 22:21
[PATCH v2] system: Assert, that user and group names are unique.
(address . 45570@debbugs.gnu.org)
20210106212148.28720-1-leo.prikler@student.tugraz.at
*gnu/system/shadow.scm (find-duplicates): New variable.
(assert-unique-account-names, assert-unique-group-names): New variables.
(account-activation): Use them here.
---
gnu/system/shadow.scm | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

Toggle diff (72 lines)
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..4dbd578e1e 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -20,6 +20,7 @@
;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
(define-module (gnu system shadow)
+ #:use-module ((guix diagnostics) #:select (formatted-message))
#:use-module (guix records)
#:use-module (guix gexp)
#:use-module (guix store)
@@ -34,6 +35,7 @@
#:use-module ((gnu packages admin)
#:select (shadow))
#:use-module (gnu packages bash)
+ #:use-module (ice-9 match)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
@@ -222,6 +224,40 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
(rename-file ".nanorc" ".config/nano/nanorc"))
#t))))
+(define (find-duplicates list)
+ (let loop ((table (make-hash-table))
+ (list list))
+ (match list
+ (()
+ (hash-fold (lambda (key value seed)
+ (if (> value 1)
+ (cons key seed)
+ seed))
+ '()
+ table))
+ ((first . rest)
+ (hash-set! table first
+ (1+ (hash-ref table first 0)))
+ (loop table rest)))))
+
+(define (assert-unique-account-names users)
+ (match (find-duplicates (map user-account-name users))
+ (() *unspecified*)
+ (duplicates
+ (raise
+ (formatted-message
+ (G_ "the following accounts appear more than once:~{ ~a~}~%")
+ duplicates)))))
+
+(define (assert-unique-group-names groups)
+ (match (find-duplicates (map user-group-name groups))
+ (() *unspecified*)
+ (duplicates
+ (raise
+ (formatted-message
+ (G_ "the following groups appear more than once:~{ ~a~}~%")
+ duplicates)))))
+
(define (assert-valid-users/groups users groups)
"Raise an error if USERS refer to groups not listed in GROUPS."
(let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +328,8 @@ group."
(define group-specs
(map user-group->gexp groups))
+ (assert-unique-account-names accounts)
+ (assert-unique-group-names groups)
(assert-valid-users/groups accounts groups)
;; Add users and user groups.
--
2.30.0
L
L
Ludovic Courtès wrote on 7 Jan 2021 09:29
Re: bug#45570: [PATCH] system: Assert, that user and group names are unique.
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87im89jgjc.fsf@gnu.org
Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

Toggle quote (30 lines)
> Am Mittwoch, den 06.01.2021, 14:32 +0100 schrieb Ludovic Courtès:
>> Hi,
>>
>> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>>
>> > > > + ((first . rest)
>> > > > + (if (member first rest =) ; (srfi srfi-1) member
>> > > > + (cons first (find-duplicates rest =))
>> > > > + (find-duplicates rest =)))))
>> > >
>> > > Note that this is quadratic; it’s fine as long as we don’t have
>> > > “too
>> > > many” users, which may be the case in general.
>> > It is indeed quadratic, but would there even be an n log n
>> > solution?
>> > I've once done an n log n sort+delete-duplicates!, perhaps that'd
>> > be a
>> > nicer solution here?
>>
>> You could first build a hash table or vhash or set with all the
>> names,
>> then traverse again the list of names and check whether they’re in
>> that
>> table. That’d be linear (assuming the table is well balanced), but
>> the
>> constant factor would be higher.
> Yeah, I think the hash table solution would make the most sense here.
> Since VHashes are based on VLists, they're not actually purely
> functional, are they?

Their implementation is not “purely functional” but it’s
inconsequential; it’s a persistent data structure, and that’s what
matters (info "(guile) VLists").

Toggle quote (16 lines)
>> > > (define (assert-unique-account-names users)
>> > > (match (find-duplicates things …)
>> > > (() #t)
>> > > (lst
>> > > (raise (formatted-message (G_ "the following accounts
>> > > appear
>> > > more than once:~{ ~a~}~%"
>> > > lst))))))
>> > >
>> > > ?
>> > That'd be weird for duplicate duplicates, hence just reporting the
>> > first.
>>
>> You could do (delete-duplicates lst) in the message above?
> Sure, but that'd be O(n^2) on top of O(n^2), which is less than ideal.

Yes, but it’s a small ‘n’, typically one or two.

Ludo’.
L
L
Ludovic Courtès wrote on 7 Jan 2021 09:35
Re: [PATCH v2] system: Assert, that user and group names are unique.
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87czyhjg9s.fsf@gnu.org
Leo Prikler <leo.prikler@student.tugraz.at> skribis:

Toggle quote (4 lines)
> *gnu/system/shadow.scm (find-duplicates): New variable.
> (assert-unique-account-names, assert-unique-group-names): New variables.
> (account-activation): Use them here.

Final nitpicks! :-)

Toggle quote (2 lines)
> +(define (find-duplicates list)

Please add a docstring.

Toggle quote (3 lines)
> + (let loop ((table (make-hash-table))
> + (list list))

You can move ‘table’ out of the ‘loop’ arguments since it’s mutated
anyway.

OK with these changes!

Thanks,
Ludo’.
L
L
Leo Prikler wrote on 7 Jan 2021 12:10
[PATCH v3] system: Assert, that user and group names are unique.
(address . 45570@debbugs.gnu.org)
20210107111019.7277-1-leo.prikler@student.tugraz.at
*gnu/system/shadow.scm (find-duplicates): New variable.
(assert-unique-account-names, assert-unique-group-names): New variables.
(account-activation): Use them here.
---
gnu/system/shadow.scm | 44 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

Toggle diff (78 lines)
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index a69339bc07..183b2cd387 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -20,6 +20,7 @@
;;; along with GNU Guix. If not, see <http://www.gnu.org/licenses/>.
(define-module (gnu system shadow)
+ #:use-module ((guix diagnostics) #:select (formatted-message))
#:use-module (guix records)
#:use-module (guix gexp)
#:use-module (guix store)
@@ -34,6 +35,7 @@
#:use-module ((gnu packages admin)
#:select (shadow))
#:use-module (gnu packages bash)
+ #:use-module (ice-9 match)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-26)
#:use-module (srfi srfi-34)
@@ -222,6 +224,46 @@ for a colorful Guile experience.\\n\\n\"))))\n"))
(rename-file ".nanorc" ".config/nano/nanorc"))
#t))))
+(define (find-duplicates list)
+ "Find duplicate entries in @var{list}.
+Two entries are considered duplicates, if they are @code{equal?} to each other.
+This implementation is made asymptotically faster than @code{delete-duplicates}
+through the internal use of hash tables."
+ (let loop ((list list)
+ ;; We actually modify table in-place, but still allocate it here
+ ;; so that we only need one level of indentation.
+ (table (make-hash-table)))
+ (match list
+ (()
+ (hash-fold (lambda (key value seed)
+ (if (> value 1)
+ (cons key seed)
+ seed))
+ '()
+ table))
+ ((first . rest)
+ (hash-set! table first
+ (1+ (hash-ref table first 0)))
+ (loop rest table)))))
+
+(define (assert-unique-account-names users)
+ (match (find-duplicates (map user-account-name users))
+ (() *unspecified*)
+ (duplicates
+ (raise
+ (formatted-message
+ (G_ "the following accounts appear more than once:~{ ~a~}")
+ duplicates)))))
+
+(define (assert-unique-group-names groups)
+ (match (find-duplicates (map user-group-name groups))
+ (() *unspecified*)
+ (duplicates
+ (raise
+ (formatted-message
+ (G_ "the following groups appear more than once:~{ ~a~}")
+ duplicates)))))
+
(define (assert-valid-users/groups users groups)
"Raise an error if USERS refer to groups not listed in GROUPS."
(let ((groups (list->set (map user-group-name groups))))
@@ -292,6 +334,8 @@ group."
(define group-specs
(map user-group->gexp groups))
+ (assert-unique-account-names accounts)
+ (assert-unique-group-names groups)
(assert-valid-users/groups accounts groups)
;; Add users and user groups.
--
2.30.0
L
L
Leo Prikler wrote on 7 Jan 2021 12:13
Re: [PATCH v2] system: Assert, that user and group names are unique.
(name . Ludovic Courtès)(address . ludo@gnu.org)
08d83d415bc59d7b86dc7233f48ece7d909069f2.camel@student.tugraz.at
Am Donnerstag, den 07.01.2021, 09:35 +0100 schrieb Ludovic Courtès:
Toggle quote (12 lines)
> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>
> > *gnu/system/shadow.scm (find-duplicates): New variable.
> > (assert-unique-account-names, assert-unique-group-names): New
> > variables.
> > (account-activation): Use them here.
>
> Final nitpicks! :-)
>
> > +(define (find-duplicates list)
>
> Please add a docstring.
Done, see v3.

Toggle quote (5 lines)
> > + (let loop ((table (make-hash-table))
> > + (list list))
>
> You can move ‘table’ out of the ‘loop’ arguments since it’s mutated
> anyway.
I don't see any benefit from doing so, however. It'd be an additional
layer of mutation and if we ever wanted to change to vhashes or alists
we'd have to refactor that.

Regards,
Leo
L
L
Ludovic Courtès wrote on 11 Jan 2021 14:09
Re: [PATCH v3] system: Assert, that user and group names are unique.
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87eeirfwma.fsf@gnu.org
Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

Toggle quote (4 lines)
> *gnu/system/shadow.scm (find-duplicates): New variable.
> (assert-unique-account-names, assert-unique-group-names): New variables.
> (account-activation): Use them here.

LGTM, thanks! :-)

Ludo’.
L
L
Leo Prikler wrote on 11 Jan 2021 16:06
(name . Ludovic Courtès)(address . ludo@gnu.org)
1ef9ad81776e110cb538968f2a124df74bec06ac.camel@student.tugraz.at
Am Montag, den 11.01.2021, 14:09 +0100 schrieb Ludovic Courtès:
Toggle quote (12 lines)
> Hi,
>
> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>
> > *gnu/system/shadow.scm (find-duplicates): New variable.
> > (assert-unique-account-names, assert-unique-group-names): New
> > variables.
> > (account-activation): Use them here.
>
> LGTM, thanks! :-)
>
> Ludo’.
Aaaand it's pushed.
Closed
?