[Libguestfs] [nbdkit PATCH v2 1/3] server: Add nbdkit_stdio_safe
Eric Blake
eblake at redhat.com
Tue Apr 14 14:01:54 UTC 2020
On 4/14/20 2:47 AM, Richard W.M. Jones wrote:
> On Mon, Apr 13, 2020 at 07:28:59PM -0500, Eric Blake wrote:
> [...]
>
> This patch is fine and can be pushed if you want, but I've got some
> small comments.
>
>> +If C<nbdkit_stdio_safe> returns true, the value of the configuration
>> +parameter may be used to trigger reading additional data through stdin
>> +(such as a password or inline script).
>
> I wonder if we want to say "returns 1" rather than true, to give
> ourselves wiggle room in future in case we suddenly decided that we
> needed this to return an error indication? On the other hand, maybe
> errors can never happen in any conceivable situation.
Sure, I can clean up the wording.
>
>> @@ -455,6 +467,10 @@ nbdkit_read_password (const char *value, char **password)
>>
>> if (nbdkit_parse_int ("password file descriptor", &value[1], &fd) == -1)
>> return -1;
>> + if (!nbdkit_stdio_safe () && fd < STDERR_FILENO) {
>
> I think this could be clearer written the other way around:
>
> if (fd < STDERR_FILENO && !nbdkit_stdio_safe ()) {
>
> but then thinking about this more, why isn't it this?
>
> if (fd == STDIN_FILENO && !nbdkit_stdio_safe ()) {
nbdkit_stdio_safe controls not only whether you should avoid reading
stdin, but also whether you should avoid writing to stdout. Then again,
no one in their right mind tries to read a password from stdout (even
when it is opened bi-directionally), and nbdkit_read_password is only
about reading. So limiting to fd == STDIN_FILENO doesn't seem too bad.
>
> Anyway, these are minor points, ACK.
>
> Rich.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
More information about the Libguestfs
mailing list