9fans / plan9port

Plan 9 from User Space
https://9fans.github.io/plan9port/
Other
1.61k stars 319 forks source link

grep can't match \r #583

Closed 9gay closed 1 year ago

9gay commented 1 year ago

screenshot

dancrossnyc commented 1 year ago

I cannot reproduce this on either plan9 or plan9ports. I suspect something in xargs and 9.rc may be messing things up.

9gay commented 1 year ago

@dancrossnyc alpine linux 3.16, plan9port-0_git20211019-r1, screenshot

9gay commented 1 year ago

@dancrossnyc maybe it has to do something with musl libc

dancrossnyc commented 1 year ago

I've tried on Plan 9, OpenBSD, a couple of varieties of Linux (Arch and Debian, though not Alpine) and macOS, and sadly I cannot reproduce this. Note that the string 'syntax error' does not appear in the grep source code or related plan9 libraries, and indeed, the few places that it appears in plan9port source code at all mostly seem unrelated, save one spot in rc. But, I can't reproduce your error under rc either. There is a call to yyerror("syntax"); in src/cmd/grep/sub.c, but that doesn't append the string "error". You may want to consider editing that file and putting an assertion or debug print in str2top to see if that's triggering it?

9gay commented 1 year ago

@dancrossnyc you're right, it's not grep's bug (my printf wasn't triggered), but rather rc's bug screenshot

dancrossnyc commented 1 year ago

I don't see how that's a bug...? The screenshot is cropped (pasting text would be much more helpful, honestly) but it appears you tried to run a command called CR and rc is simply telling you that no such thing exists.

9gay commented 1 year ago

@dancrossnyc I'm not pasting text simply because I can't type that char here, but never mind attempt to run it as command and instead pay attention to the fact that rc printed "\n'\n\t\n" before ": No such file or directory\n"

9gay commented 1 year ago
% echo '
'

'

% 

"% echo '\r'\n\n'\n\t\n\n% "

dancrossnyc commented 1 year ago

I think that's an artifact of whatever windowing environment you are in. Here's running rc under iTerm2 on macOS:

% ^M
: No such file or directory
%

Again, I don't think there's an rc bug here. I don't see how it relates to your grep problem, either. Note that actual plan9 has a different failure mode, as the system enforces that CR cannot be part of a file name:

brahma% ./
./
: bad character in file name: './
'
brahma% 
9gay commented 1 year ago

windowing environment is x11 on alpine linux, this is reproducible under both win and 9term (yes I seriously use those instead of terminal emulators)

dancrossnyc commented 1 year ago

I still don't see an rc bug here. 9term's output in this context, while perhaps unexpected, isn't unreasonable: looking at the source code, it apperas that 9term explicitly converts CR's to NL's (see the function dropcr in src/cmd/9term/win.c), hence the seeming doubling that you're seeing: the echo of the character by the shell, then the error, followed by a newline, with 9term's interposition.

In other words, this is the expected behavior of the program. One might argue that that shouldn't be the behavior, and one may be right, but by definition it is not a bug.

9gay commented 1 year ago

how is this an expected behavior if you can't reproduce it on Plan 9?

dancrossnyc commented 1 year ago

The author explicitly wrote it that way.

9gay commented 1 year ago

alright, but what about acme's win? this happens only in plan9port

9gay commented 1 year ago

ah, acme and 9term share win.c

jxy commented 1 year ago

It's easy to reproduce 'syntax error' from grep. You just press g r e p [space] [quote] [enter] [quote] [enter]

% grep '
    '
grep: syntax error

which gdb shows coming from this line https://github.com/9fans/plan9port/blob/ffbdd1aa20c8a20a8e9dcd3cec644b6dfa3c6acb/src/cmd/grep/grep.y#L123 which is called by yyparse, which is invoked here https://github.com/9fans/plan9port/blob/ffbdd1aa20c8a20a8e9dcd3cec644b6dfa3c6acb/src/cmd/grep/sub.c#L161

basically grep sees a single argument of \n and can't deal with it.

is the parsing of \r rc's issue? no. check this,

% xd -c tmp/test_rc
0000000   #  !  /  u  s  r  /  b  i  n  /  e  n  v     r
0000010   c \n  g  r  e  p     ' \r  '     $  0 \n
000001e 
% ./tmp/test_rc|xd -c
0000000   g  r  e  p     ' \r  '     $  0 \n
000000c 

so it looks like to me it's just 9term trying to be smart, eats the \r and spits out a \n.

dancrossnyc commented 1 year ago

Yup. That's exactly right.

9gay commented 1 year ago

@jxy I'm unable to reproduce that one, it just matches every newline which almost makes it replacement to cat