Hi,
On Thu, Apr 17, 2025 at 01:32:55PM +0200, THIBAUT AUBIN wrote:
> I respectfully disagree with Roberto Vargas.
> NRK suggested approach is consistent with the behavior of GNU coreutils
> 8.30.
Well, disagree is a good thing, and I would like more constructive
discussion in the mailing list, but using coreutils like an authoraty
principle seems wrong to me. If you want coreutils you know where
to find them, and the main reason why sbase exists is because
coreutils exits and sucks.
> The first character from stdin should be obtained directly, without reading
> any leading characters.
> This allows all the remaining characters to be consumed within a single
> loop.
This is not what the patch from NRK did. Quoting his own words:
The whole line needs to be read, even when the first char
is `y|Y`. Also, getchar may return EOF before newline
leading to an infinite loop.
So, it was using the first char, but then ignoring everything else from
stdin until new line. Let me put here again his patch:
int ans = getchar();
for (int c = ans; c != '\n' && c != EOF;)
c = getchar();
if (ans != 'Y' && ans != 'y')
return 0;
The for loop consumes the full line after the initial chararcter, being
spaces or not.
> Furthermore, it is also necessary to check for both newline and EOF from
> the start to prevent an infinite loop.
No, if you only want to avoid the infinite loop then you only have
to care about the EOF. The problem here is if you don't consume the
newline then the next getchar will get the newline. This is why NRK
was proposing the line approach, that in general is better for
interactive situations like this case (and in fact, it is mandated
by POSIX as explained later in this mail).
> All trailing characters must be consumed, using `isspace(3)` is incorrect.
Can you specify in which section of POSIX did you get that sentence?
In the version of POSIX that I have it says:
If the -i option is specified, rm shall write a prompt to
standard error and read a line from the standard input. If
the response is not affirmative, rm shall do nothing more
with the current file, and go on to any remaining files.
but it does not specify what is an affirmative response. In fact,
the following code fulfil that requirement (no error checks only
for simplicity):
char line[81], dummy;
fgets(line, sizeof(line), stdin);
if (sscanf(" [yY]%c", &dummy) == 1)
return 1;
I am going to highlight the important part: *and read a line*.
As the standard does not say anything about how to deal with
spaces, the more logical thing is to skip them. Implementations
that don't do that are just stupid and they don't add value to the
user, and they only confuse him:
' y\n' -> negative
'y \n' -> positive
Just apply the Principle of least astonishment [1] and skip spaces.
> Since the answer may be either `y` or `Y`, using `toupper(3)` is incorrect.
Why toupper is incorrect? toupper('y') == 'Y' and toupper('Y') == 'Y' and
toupper of non printable characters are themself. If toupper returns 'Y'
it is because the input was 'y' or 'Y'. From the POSIX standard (that just
copies verbatim the C standard):
If the argument of toupper() represents a lowercase letter,
and there exists a corresponding uppercase letter as defined
by character type information in the current locale
respectively (category LC_CTYPE), the result shall be the
corresponding uppercase letter.
All other arguments in the domain are returned unchanged.
so, `touper(c) == 'Y'` is equivalent (and usually more efficient in space
and speed) to `c == 'y' || c == 'Y'`.
> Unless there are issues with this implementation, it's time to apply patch
> sbase-20250416-8f0fff4.diff.
As you can see, there are many topics about the patch. I would like to hear
more opinions about the topic, specially the opinion of NRK.
Regards,
[1] https://en.wikipedia.org/wiki/Principle_of_least_astonishment
