Re: [PATCH] yesno: Fix behavior difference in ENABLE_NLS

[Pádraig Brady]

On 23/03/15 22:09, Tobias Stoeckmann wrote:
> yesno behaves differently in a corner case depending on ENABLE_NLS.
> Imagine an input of "y" followed by an EOF. On a terminal, it means
> in general to type y followed by ^D once or twice.
> 
> If ENABLE_NLS is set, the input is considered to be "no", because the
> last character is replaced with '\0'. The code assumes that there is
> a newline, which doesn't have to be true. In this example, it overrides
> the typed y before evaluating the line.
> 
> If ENABLE_NLS is not set, getchar() reads y and accepts it as "yes",
> looping through more getchar() calls until reaching newline or EOF.
> 
> I recommend to just skip the '\0' assignment. Less code than checking
> if newline is in place or not, because rpmatch() only checks first
> character.
> ---
>  lib/yesno.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/yesno.c b/lib/yesno.c
> index d5d9eec..0f7bf76 100644
> --- a/lib/yesno.c
> +++ b/lib/yesno.c
> @@ -42,7 +42,6 @@ yesno (void)
>      yes = false;
>    else
>      {
> -      response[response_len - 1] = '\0';
>        yes = (0 < rpmatch (response));
>      }

Yes this is better since getline() is guaranteed to NUL terminate in this path.
BTW I presume getline() on windows transforms "\r\n" to '\n'.

However this changes the behavior for uk_UA.utf8 here due to:

  $ LC_ALL=uk_UA.utf8 locale yesexpr
  ^([Yy+]|[Тт][Аа][Кк]?)$

I.E. that locale is just allowing the single 'y' to be affirmative,
rather than 'yes' etc.

It's probably better to honor that restriction and conditionally
remove the \n like:

    if (response[response_len - 1] == '\n')
      response[response_len - 1] = '\0';

cheers,
Pádraig.