[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Chicken-hackers] [PATCH] Make process procedures in the posix modul
From: |
Evan Hanson |
Subject: |
Re: [Chicken-hackers] [PATCH] Make process procedures in the posix module accept alists for environments. |
Date: |
Fri, 10 Mar 2017 10:25:36 +1300 |
Hi Kooda,
Very nice, I like this change.
On 2017-03-02 0:40, Kooda wrote:
> diff --git a/posix-common.scm b/posix-common.scm
> index f8fe27fa..f3d444c9 100644
> --- a/posix-common.scm
> +++ b/posix-common.scm
> @@ -758,6 +768,9 @@ EOF
>
> ;; Envlist is never converted, so we always use nop here
> (when envlist
> - (set! envbuf (list->c-string-buffer envlist nop loc)))
> + (set! envbuf
> + (list->c-string-buffer
> + (map (lambda (p) (string-append (car p) "=" (cdr p))) envlist)
> + nop loc)))
I think `call-with-exec-args` should use `check-environment-list` here,
before you build the list of "KEY=VALUE" strings, or should at least do
some equivalent checking. Otherwise, when given a bad envlist, the error
message will be issued by `car` or `string-append`, which is less clear
than in `process` where the argument check will use the right location
for error reporting:
#;> (process-execute "/bin/ls" '() '("abc"))
Error: (car) bad argument type: "abc"
vs.
#;3> (process "/bin/ls" '() '("abc"))
Error: (process) bad argument type - not a pair: "abc"
Otherwise the patch LGTM.
Cheers,
Evan