chicken-hackers
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]