Re: [PATCH v2 2/3] parser: synerror: explicitly consume the entire invalid line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 04/01/2023 09:51, Herbert Xu wrote:
On Tue, Jan 03, 2023 at 11:47:05AM +0000, Harald van Dijk wrote:

In order for this to be a problem, we need something where the newline
itself triggers a syntax error. For synexpect(TDO), is that possible? In all
cases where synexpect(TDO) is called, a newline is permitted, and it is the
token after the newline that the error will be reported on, no? In that
situation, reading the rest of the line is correct.

You're right, it isn't possible with TDO.

I'm not going to rule out that there is a potential for problems, but if
there is, it's not the situation you say.

However, the synexpect in parsefname would seem to qualify:

cat <<- <newline>

You're right, that does show the problem, thanks.

Since the only case where special care is needed is when the problematic token was newline, it can be handled as simply as

  if (lasttoken != TNL)
    skipline();

I will do some testing to see if I am overlooking something, and follow up if I am.

Handling it in the input layer after the error has already been reported
means we get inconsistent error handling when running with dash -isv. The
error may, depending on whether the rest of the line was in the buffer
already, be reported either in the middle of the input line, or on the next
line.

Just try changing sh -i to sh -iv in the reproducer and seeing what what
happens with your patch.

I don't think this is such a big deal.  You get the same effect
if you simply do input line buffering but only up to a certain
length.  I tried one million instead of ten thousand with the
reproducer and ksh93 prints out the error after only 65536
characters.

In any case, we could actually fix this by buffering stderr and
then making sh -v bypass the buffer and write directly to 2.

I tried it and it actually seems to work.  But all the extra flush
calls bloat up the code a bit so it needs a bit more work before
I'm happy to run with it.

Interesting approach. Error messages are known to always fit in the buffer, so you can probably be sure it won't be forced to be emitted early. It could work.

Cheers,
Harald van Dijk



[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux