[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [SHEPHERD] [PATCH] service: Improve 'service-list'.
From: |
Ludovic Courtès |
Subject: |
Re: [SHEPHERD] [PATCH] service: Improve 'service-list'. |
Date: |
Mon, 25 Jan 2016 22:49:47 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Alex Kost <address@hidden> skribis:
> Ludovic Courtès (2016-01-25 18:06 +0300) wrote:
>
>> Alex Kost <address@hidden> skribis:
>>
>>> From f3d21e3ec8a100a966153d03264639ebe48e8872 Mon Sep 17 00:00:00 2001
>>> From: Alex Kost <address@hidden>
>>> Date: Mon, 25 Jan 2016 11:18:00 +0300
>>> Subject: [PATCH] service: Improve 'service-list'.
>>>
>>> * modules/shepherd/service.scm (service-list): Use
>>> 'lookup-canonical-service' on each name instead of removing duplicates
>>> from the final list.
>>
>> [...]
>>
>>> diff --git a/modules/shepherd/service.scm b/modules/shepherd/service.scm
>>> index f84d1dd..94f2aae 100644
>>> --- a/modules/shepherd/service.scm
>>> +++ b/modules/shepherd/service.scm
>>> @@ -871,12 +871,13 @@ Return #f if service is not found."
>>>
>>> (define (service-list)
>>> "Return the list of services currently defined."
>>> - (delete-duplicates
>>> - (hash-fold (lambda (key services result)
>>> - (append services result))
>>> - '()
>>> - %services)
>>> - eq?))
>>> + (hash-fold (lambda (name services result)
>>> + (let ((service (lookup-canonical-service name services)))
>>> + (if service
>>> + (cons service result)
>>> + result)))
>>> + '()
>>> + %services))
>>
>> OK, except that we know that SERVICE is necessarily true, because the
>> canonical service for NAME is necessarily among SERVICES.
>>
>> So I would remove the ‘if’ and add a comment explaining the above.
>>
>> OK with this change?
>
> No, the service is not necessarily true. When a service has several
> names (e.g., "root" and "shepherd"), then %services table will contain 2
> entries (with 'root' and 'shepherd' keys and the same (#<<service> …>)
> value). So for one of the hash-table entries:
>
> (lookup-canonical-service 'root (list root-service))
>
> returns #t, and for the other:
>
> (lookup-canonical-service 'shepherd (list root-service))
>
> it returns #f.
Indeed, my bad.
Then OK to push, thanks!
Ludo’.
- [PATCH 3/5] service: 'service-list' returns unique services., (continued)
- [PATCH 3/5] service: 'service-list' returns unique services., Alex Kost, 2016/01/24
- [PATCH 4/5] service: Add docstring to 'lookup-services'., Alex Kost, 2016/01/24
- [PATCH 1/5] service: Rename 'services' variable to '%services'., Alex Kost, 2016/01/24
- [PATCH 5/5] Rename 'dmd' service to 'root'., Alex Kost, 2016/01/24
- Re: [SHEPHERD] [PATCH 0/5] Replace 'dmd' service with 'root/shepherd'., Ludovic Courtès, 2016/01/24