Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Goto gets NCA error when buffer is empty #21

Open
LdBeth opened this issue Feb 9, 2024 · 3 comments · May be fixed by #26
Open

Goto gets NCA error when buffer is empty #21

LdBeth opened this issue Feb 9, 2024 · 3 comments · May be fixed by #26

Comments

@LdBeth
Copy link

LdBeth commented Feb 9, 2024 •
edited
Loading

I found the bug when testing part of squ.tes from TECOC lib, below has a minimal example to reproduce the error.

The conditions needed to reproduce the bug I found are:

  • The goto and label is inside a macro register
  • ^YK has been called inside the macro level
  • When the goto O is called the buffer is empty

In most of the case in squ.tes when the macro is called the buffer wouldn't be empty, but I have tested the same example in TECOC and it
has no problem even when the buffer is empty.

As a side note rewrite goto using loop '< >' can work around this issue.

This bug reproduces both on macOS (built with Clang) and Linux (built with GCC).

*eitest.tes``

Delete CR/LF (Y/N) <N>? 12^U?NCA   Negative argument to comma

^D                              ! set decimal !
0ED                             ! set edit mode zero !
0^X                             ! set search mode zero !
0,128ET                         ! set abort-on-error !

! R get Response from user !
@^UR*
^YX0
^YK                             ! kill last inserted string XXX: if move outside macro won't reproduce !
.U1                             ! num 1 <- current point !
ZJ                              ! go to end !
.U2                             ! num 2 <- save buffer end pos !

! display the prompt on a new, clean, line !

!PMPT!

13^T10^T                        ! type <CR><LF> !
        :G0                     !   type str 0 !
        Q2,ZT                    !   and input so far !


!GETCH!
^TU0                            ! read char to num 0 !
Q0-21 "E                        ! if char = ^U !
        Q2,ZK                   !  XXX: when the buffer is empty !
                                !  XXX: the goto would fail by ?NCA !
        @O!PMPT!                !   go to PROMPT !
'                               ! fi !
Q0-27 "E                        ! if char = <ESC> !
        13^T 10^T               !   type <CR> <LF> !
        @O!DONE!                !   go to DONE XXX: if input is empty this would also fail !
'                               ! fi !
Q0@I//                          ! insert input char to buffer !
@O!GETCH!                       ! go to GETCH !

!DONE!

Q1J                             ! restore to original buffer position !
*                               ! end of ^UR !

@I+Delete CR/LF (Y/N) <N>? +    ! str 0 <- display prompt !

MR
HT
^[^[
@LdBeth
Copy link
Author

LdBeth commented Feb 10, 2024 •
edited
Loading

Ok I have did some debug with lldb and found the NCA is caused by confirm_cmd() in cmd_exec.c and although I do not fully understand how the command syntax check works, I think the problem is when go to is used inside a macro TECO-64 scans the text before the label and thinks ^Y gives a negative range for the current buffer is empty when it it actually valid syntax and won't be executed anyway. I guess some of the program logic is not fully correct there, but I believe it is not difficult to fix by adding a different check option when looking for go to labels.

@LdBeth LdBeth changed the title Goto inside macro receives NCA error when buffer is empty Goto gets NCA error when buffer is empty Feb 10, 2024
@LdBeth
Copy link
Author

LdBeth commented Feb 11, 2024 •
edited
Loading

So the good news is by rebuild TECO-64 with nstrict=1, I can get rid of the NCA error and I have changed enough of squ.tes to have it handle alternative style comment and paired delimiter (only works for ^U), but that's enough to bootstrap the modified the version of squ.tes, that is make it squish itself and the squished version works identically to unquished itself. And I think filling the rest of the gap in the language (ignore space when looking for delimiter, handle ! in enclosed parentheses, etc) is not that difficult.

Here is a tiny benchmark:

$ time teco -T 'test.tec=sqush.tes/D:Y/B:Y/C:+ :/T:Y/E:Y/P/A:Y' -E sqush.tec
Squishing "/Users/ldbeth/sqush/sqush.tes
Superseding existing file 'test.tec'
Creating "/Users/ldbeth/sqush/test.tec
    0m04.13s real     0m04.13s user     0m00.00s system
$ time teco -T 'test.tec=sqush.tes/D:Y/B:Y/C:+ :/T:Y/E:Y/P/A:Y' -E sqush.tes

Squishing "/Users/ldbeth/sqush/sqush.tes
Superseding existing file 'test.tec'
Creating "/Users/ldbeth/sqush/test.tec
    0m30.61s real     0m30.59s user     0m00.00s system

$ ls -lh sqush.*
-rw-r--r--@ 1 ldbeth  staff   4.8K Feb 11 14:46 sqush.tec
-rw-r--r--  1 ldbeth  staff    88K Feb 11 14:44 sqush.tes

@LdBeth
Copy link
Author

LdBeth commented Feb 17, 2024 •
edited
Loading

So now I have understood how the confirm_cmd works, I found the cause is pretty obvious:

The goto command uses skip_cmd() to scan for labels, which calls scan_cmd() and dispatches to scan_*() functions
to consume command arguments, but for many commands, the confirm assertion has been put to the scan_*() part instead of exec_*() part, the X is one of them.

The fix is then very obvious, moving all the confirm checks to the negativity of the command to the exec_*() function.

@LdBeth LdBeth linked a pull request Feb 17, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Postpone negativity check to runtime LdBeth/TECO-64
1 participant
@LdBeth