Re: [Bug-wget] [PATCH v2] wget: Add --use-askpass support

[Ángel González]

On 25/07/16 20:28, Liam R. Howlett wrote:
> This adds the --use-askpass option which is disabled by default.
>
> --use-askpass will request the username and password for a given URL by
> executing the external program pointed to by the environment variable
> WGET_ASKPASS.  If the environment variable is not set, SSH_ASKPASS is
> used.  If neither environment variable are set, an error is returned.
> If an error occurs requesting the username or password, wget will exit.
>
> Changes from V1:
>   - Removed debug strace command.
>   - Removed fork in favour of posix_spawnp.
>   - Changed option from --ssh-askpass to --use-askpass.
>   - Check WGET_ASKPASS then fall back to SSH_ASKPASS.
>   - Updated documentation
>
> Liam R. Howlett (1):
>    wget: Add --use-askpass support

I like that better. Still, perhaps allowing the parameter to provide the 
binary would be beneficial for configurating in .wgetrc instead of 
requiring an env variable.

There are a few spots that require C99 (char * const argv, unsigned int 
bytes), but I'm unsure if we ending up dropping C90 support or not.

There's a possible overflow of question in use_askpass sprintf calls. I 
recommend you to change it to snprintf. Also, those prompts should be 
wrapped with _() for localisation.

If you use posix_spawnp, you don't call execlp.

There are a few spacing issues. Eg. compare your getenvs.

Regards