From: "Ludovic Courtès" <ludo@gnu.org>
To: Giacomo Leidi <goodoldpaul@autistici.org>
Cc: 72337@debbugs.gnu.org
Subject: [bug#72337] Add /etc/subuid and /etc/subgid support
Date: Wed, 04 Sep 2024 23:00:58 +0200 [thread overview]
Message-ID: <878qw7b0c5.fsf_-_@gnu.org> (raw)
In-Reply-To: <5b955b5c53e8e2c7c3173c87ca17758505e960ae.1724192097.git.goodoldpaul@autistici.org> (Giacomo Leidi's message of "Wed, 21 Aug 2024 00:14:56 +0200")
Giacomo Leidi <goodoldpaul@autistici.org> skribis:
> * gnu/build/accounts.scm (list-set): New variable;
> (%sub-id-min): new variable;
> (%sub-id-max): new variable;
> (%sub-id-count): new variable;
> (sub-id?): new variable;
> (subid-range-fits?): new variable;
> (subid-range-fits-between?): new variable;
> (insert-subid-range): new variable;
> (reserve-subids): new variable;
> (range->entry): new variable;
> (entry->range): new variable;
> (allocate-subids): new variable;
> (subuid+subgid-databases): new variable.
>
> * gnu/system/accounts.scm (subid-range-end): New variable;
> (subid-range-has-start?): new variable;
> (subid-range-less): new variable.
>
> * test/accounts.scm: Test them.
>
> Change-Id: I8de1fd7cfe508b9c76408064d6f498471da0752d
Woow, neat! It didn’t occur to me that we’d need a proper subid
allocation mechanism as well.
> +(define (list-set lst el k)
> + (if (>= k (length lst))
> + `(,@lst ,el)
> + `(,@(list-head lst k)
> + ,el
> + ,@(list-tail lst k))))
‘length’, ‘list-ref’, and thus ‘list-set’ are linear in the size of the
list so it’s something we should avoid, unless we know that the lists
we’re dealing with are always going to be small.
> +;; According to Shadow's libmisc/find_new_sub_uids.c and
> +;; libmisc/find_new_sub_gids.c.
> +(define %sub-id-min 100000)
> +(define %sub-id-max 600100000)
> +(define %sub-id-count 65536)
[...]
> +(define (sub-id? id)
> + (and (>= id %sub-id-min)
> + (< id %sub-id-max)))
s/sub-/subordinate-/
> +(define (subid-range-fits? r interval-start interval-end)
> + (and (<= interval-start
> + (subid-range-start r))
> + (<= (subid-range-end r)
> + interval-end)))
Maybe: (within-subordinate-id-range? start end range) ?
Also, shouldn’t the first <= be >= ?
Please add docstrings for top-level procedures.
> +(define (subid-range-fits-between? r a b)
> + (subid-range-fits? r
> + (+ (subid-range-start a) 1)
> + (- (subid-range-end b) 1)))
Maybe: (containing-subordinate-id-range? range a b) ?
> +(define (insert-subid-range range lst)
We definitely need a docstring, I’m not sure what this is supposed to
do. :-)
> + (unless (and (sub-id? range-start)
> + (sub-id? range-end))
> + (raise
> + (string-append "Subid range of " range-name
> + " from " (number->string range-start) " to "
> + (number->string range-end)
> + " spans over illegal subids. Max allowed is "
> + (number->string %sub-id-max) ", min is "
> + (number->string %sub-id-min) "."))))
There are two issues: first we need ‘raise’ from (srfi srfi-34), not
from (guile), since the latter has nothing to do with exceptions.
Second, ‘raise’ takes a SRFI-35 “error condition” (essentially a
record), not a string.
But my suggestion here would be to define specific error conditions,
like:
(define-condition-type &subordinate-id-error &error)
(define-condition-type &subordinate-id-range-error &subordinate-id-error
(id subordinate-id-range-error-id))
The latter is what we’d use here.
This procedure uses lists a lot, which should probably be avoided as I
wrote above. Perhaps a vlist would do, or perhaps a vhash, or a vector.
The procedure is also very long; I wonder if it could be further split
and/or share code with the existing allocation-related code.
> + (test-error "allocate-subids with interleaving, impossible interleaving"
> + "error"
> + ;; Make sure it's impossible to explicitly request impossible allocations
Instead of ‘test-error’, which is currently kinda broken IIRC, I’d
suggest a more explicit approach:
(test-assert …
(guard (c ((whatever-error? c) #t))
…
#f))
Thanks,
Ludo’.
next prev parent reply other threads:[~2024-09-04 21:02 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-28 15:25 [bug#72337] Add /etc/subuid and /etc/subgid support paul via Guix-patches via
2024-07-28 15:29 ` [bug#72337] [PATCH 1/3] accounts: " Giacomo Leidi via Guix-patches via
2024-07-28 15:29 ` [bug#72337] [PATCH 2/3] account: Add /etc/subid and /etc/subgid allocation logic Giacomo Leidi via Guix-patches via
2024-07-28 15:29 ` [bug#72337] [PATCH 3/3] system: Add /etc/subuid and /etc/subgid support Giacomo Leidi via Guix-patches via
2024-08-19 21:32 ` [bug#72337] " paul via Guix-patches via
2024-08-20 22:12 ` paul via Guix-patches via
2024-08-19 22:08 ` [bug#72337] [PATCH v2 1/3] accounts: " Giacomo Leidi via Guix-patches via
2024-08-19 22:08 ` [bug#72337] [PATCH v2 2/3] account: Add /etc/subid and /etc/subgid allocation logic Giacomo Leidi via Guix-patches via
2024-08-19 22:08 ` [bug#72337] [PATCH v2 3/3] system: Add /etc/subuid and /etc/subgid support Giacomo Leidi via Guix-patches via
2024-08-20 22:14 ` [bug#72337] [PATCH v3 1/3] accounts: " Giacomo Leidi via Guix-patches via
2024-08-20 22:14 ` [bug#72337] [PATCH v3 2/3] account: Add /etc/subid and /etc/subgid allocation logic Giacomo Leidi via Guix-patches via
2024-09-04 21:00 ` Ludovic Courtès [this message]
2024-08-20 22:14 ` [bug#72337] [PATCH v3 3/3] system: Add /etc/subuid and /etc/subgid support Giacomo Leidi via Guix-patches via
2024-09-04 21:20 ` [bug#72337] " Ludovic Courtès
2024-09-07 20:44 ` paul via Guix-patches via
2024-09-04 20:34 ` Ludovic Courtès
2024-09-07 20:51 ` [bug#72337] [PATCH v4 1/3] accounts: " Giacomo Leidi via Guix-patches via
2024-09-07 20:51 ` [bug#72337] [PATCH v4 2/3] account: Add /etc/subid and /etc/subgid allocation logic Giacomo Leidi via Guix-patches via
2024-09-19 11:14 ` [bug#72337] Add /etc/subuid and /etc/subgid support Ludovic Courtès
2024-09-07 20:51 ` [bug#72337] [PATCH v4 3/3] system: " Giacomo Leidi via Guix-patches via
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878qw7b0c5.fsf_-_@gnu.org \
--to=ludo@gnu.org \
--cc=72337@debbugs.gnu.org \
--cc=goodoldpaul@autistici.org \
/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 external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.