diff --git a/Programs/CosmOS/Apps/Edit.asm b/Programs/CosmOS/Apps/Edit.asm index 5d7764b..e332df2 100644 --- a/Programs/CosmOS/Apps/Edit.asm +++ b/Programs/CosmOS/Apps/Edit.asm @@ -75,8 +75,15 @@ start: RSTA STA.0 + RSTA + SETD.0 TooLong + STA.0 CALL loadFile + SETD.0 TooLong + LDA.0 + BNA tooLongToEdit + SETD.0 FileName SWI osPrintString SETD.0 CommaText @@ -164,6 +171,13 @@ quit: RSTA SWI osExit +tooLongToEdit: + SETD.0 TooLongText + SWI osPrintString + CALL newLine + INIA 0d1 + SWI osExit + noName: SETD.0 NoNameText SWI osPrintString @@ -194,7 +208,7 @@ insertLoop: SETD.0 EnteringText SWI osPrintString SETD.0 Entry - INIB 0d80 + INIB 0d128 SWI osReadLine INA 0x01 INIB 0x02 ; ENDED @@ -234,7 +248,7 @@ doChange: SETD.0 EnteringText SWI osPrintString SETD.0 Entry - INIB 0d80 + INIB 0d128 SWI osReadLine INA 0x01 INIB 0x02 ; ENDED @@ -600,6 +614,29 @@ splitStep: XOR BRQ splitLine + ; ---- Room for it ---- + ; + ; THERE WAS NO CHECK HERE AT ALL, and Entry is followed in memory by TextHead and + ; ArenaFree - the head of the document and the pointer the line allocator hands out. A + ; line longer than the buffer wrote characters over both, so the list head pointed into + ; the middle of the text and the allocator handed out an address inside the file. + ; + ; What that looked like: a thirty one line file opened as three, one of them cut short. + ; Then, opening it a second time, a list that led back into itself and a machine that + ; walked it for ever - the emulator still running and the machine never answering again. + ; + ; Typing a long line was always safe, because osReadLine is told how much room there is. + ; Only the file being read went unchecked, which is why a new document behaved and a + ; source file did not. + PSHA + SETD.2 EntryLength + LDA.2 + INIB 0d128 + CCF + SUB + POPA ; The character back, and Q still says whether there is room. + BRQ splitTooLong + STA.1 ; A is still the character; an ALU operation does not touch it. INCD.1 SETD.2 EntryLength @@ -627,6 +664,15 @@ splitOn: CALL takeOneOff BRI splitStep +splitTooLong: + ; NOT TRUNCATED. This is an editor: a line it shortened here would be written back + ; shortened, and the file would be damaged by having been looked at. Refusing leaves it + ; exactly as it was. + INIA 0x01 + SETD.2 TooLong + STA.2 + RET + splitLast: ; A file that does not end in a newline still has a last line in it. SETD.2 EntryLength @@ -812,13 +858,20 @@ NeedsLineText: "which line?" NoWriteText: "it would not write" +TooLongText: +"a line in it is longer than this can edit, so it has not been opened" FileName: #Reserve 0d24 Command: #Reserve 0d41 +; A hundred and twenty eight and the zero that ends it, which is what a line is everywhere +; else on this machine - the same number configuration files use, rather than a second +; answer to a question already answered. Entry: - #Reserve 0d81 + #Reserve 0d129 +TooLong: + 0x00 TextHead: 0x00 0x00 diff --git a/Programs/CosmOS/README.md b/Programs/CosmOS/README.md index 8ecdbeb..248c921 100644 --- a/Programs/CosmOS/README.md +++ b/Programs/CosmOS/README.md @@ -464,7 +464,20 @@ Typed in as bytes, checked by disassembling it back, and run. It ends with `SWI `Edit` is the first program on this machine that makes a file a person typed - every byte on every disk before it was put there by the host tool. It is line oriented in the manner of `ed`: `l` lists, `a` adds at the end, `i` and `c` and `d` take a line number, `w` writes and `q` stops. -It includes nothing but `services.asm` and `text.asm`: the filesystem and the console are the system's, asked for rather than carried. That is what brought `Edit` down from 4,941 bytes to 1,986 bytes without a line of its own logic changing - and the way that was checked is worth knowing, because the recorded output of the `cosmosEdit` test did not move by a single byte across the rewrite. +It includes nothing but `services.asm` and `text.asm`: the filesystem and the console are the system's, asked for rather than carried. That is what brought `Edit` down from 4,941 bytes to 2,157 bytes without a line of its own logic changing - and the way that was checked is worth knowing, because the recorded output of the `cosmosEdit` test did not move by a single byte across the rewrite. + +**A line it reads in is at most 128 characters**, the same length a line is everywhere else +on this machine, and a file with a longer one is refused rather than opened. Refused rather +than shortened, because this is an editor: a line cut on the way in would be written back +cut, and the file damaged by having been looked at. + +That limit was not there at all until a source file found it. The buffer is followed in +memory by the head of the document and the pointer the line allocator hands out, so a 94 +character line wrote characters over both - a 31 line file opened as 3, and opening it a +second time walked a list that led back into itself for ever, with the emulator still +running and the machine never answering again. Typing a long line was always safe, because +`osReadLine` is told how much room there is; only the file being read went unchecked, which +is why a new document behaved and a source file did not. It keeps the document as a **linked list of lines** rather than one buffer with newlines in it. Each line says where the next one is, how long it is, and then its bytes. Inserting is two pointers changed and nothing moved; with a flat buffer it would mean shifting every byte after the edit, on a machine whose only block move is a device asked politely. The price is that deleted lines are not reused, so a heavy session uses more room than the document needs and writing it out is what tidies up. diff --git a/Tests/expected/cosmosEditLong.out b/Tests/expected/cosmosEditLong.out new file mode 100644 index 0000000..bdbf6fa --- /dev/null +++ b/Tests/expected/cosmosEditLong.out @@ -0,0 +1,14 @@ +CosmOS +> hello.asm, 31 lines +> finished +> hello.asm, 31 lines +> finished +> the last program left 0, which is: it did what it was asked +finished +> a line in it is longer than this can edit, so it has not been opened +finished +> the last program left 1, which is: it did not +finished +> halted +Execution halted. +[exit 0] diff --git a/Tests/expected/cosmosTreeWrite.out b/Tests/expected/cosmosTreeWrite.out index 98230ff..dcb47c7 100644 --- a/Tests/expected/cosmosTreeWrite.out +++ b/Tests/expected/cosmosTreeWrite.out @@ -1,13 +1,13 @@ CosmOS > Folder -Edit.sbx 1986 +Edit.sbx 2157 1 file, 1 directory > loaded, starting at 4000 > note.txt, 0 lines > : : : > written, 67 bytes > finished > Folder -Edit.sbx 1986 +Edit.sbx 2157 note.txt 67 2 files, 1 directory > halted diff --git a/Tests/input/editLong.in b/Tests/input/editLong.in new file mode 100644 index 0000000..59d8f6c --- /dev/null +++ b/Tests/input/editLong.in @@ -0,0 +1,8 @@ +Edit hello.asm +q +Edit hello.asm +q +Status +Edit long.txt +Status +exit diff --git a/Tests/lint-baseline.txt b/Tests/lint-baseline.txt index 601d4eb..fe93cee 100644 --- a/Tests/lint-baseline.txt +++ b/Tests/lint-baseline.txt @@ -2,7 +2,8 @@ Programs/Boot/slotData.asm redundant-setd 1 Programs/Boot/stage1.asm redundant-setd 2 Programs/Boot/stage2.asm redundant-setd 1 Programs/CosmOS/Apps/Copy.asm redundant-setd 2 -Programs/CosmOS/Apps/Edit.asm redundant-setd 3 +Programs/CosmOS/Apps/Edit.asm redundant-assignment 1 +Programs/CosmOS/Apps/Edit.asm redundant-setd 4 Programs/CosmOS/Apps/Status.asm redundant-setd 1 Programs/CosmOS/Apps/Wander.asm redundant-setd 1 Programs/CosmOS/Assembler/Asm.asm redundant-assignment 2 diff --git a/Tests/makedisks.sh b/Tests/makedisks.sh index 6cc07f9..73bf04d 100755 --- a/Tests/makedisks.sh +++ b/Tests/makedisks.sh @@ -505,3 +505,26 @@ for app in Status Say Type Settle; do "$ROOT/Programs/CosmOS/Apps/$app.asm" -o "$WORK/$app.sbx" >/dev/null "$TOOL" put "$DISKS/status.img" "$WORK/$app.sbx" /Apps/$app.sbx >/dev/null done + +# ---- A file with lines longer than the editor's buffer ---- +# +# Edit copied a file into an 81 byte line buffer with NO BOUND, and the buffer is followed +# in memory by the head of the document and the pointer its allocator hands out. A source +# file with 94 character lines wrote characters over both: a 31 line file opened as 3, and +# opening it a second time walked a list that led back into itself for ever. +# +# Typing was always safe - osReadLine is told how much room there is - so a new document +# behaved and a source file did not, which is exactly how it was found. +"$TOOL" format "$DISKS/editlong.img" 512 4 >/dev/null +"$TOOL" mkdir "$DISKS/editlong.img" /Apps >/dev/null +for app in Edit Status; do + "$ROOT/Assembler" -I "$ROOT/Programs/CosmOS/Source" \ + "$ROOT/Programs/CosmOS/Apps/$app.asm" -o "$WORK/$app.sbx" >/dev/null + "$TOOL" put "$DISKS/editlong.img" "$WORK/$app.sbx" /Apps/$app.sbx >/dev/null +done +"$TOOL" put "$DISKS/editlong.img" "$ROOT/Programs/CosmOS/Apps/hello.asm" /hello.asm >/dev/null +# And one past what it can hold, which must be refused rather than shortened: a line this +# cut would be written back cut, and the file damaged by having been looked at. +{ printf 'short line\n'; printf 'x%.0s' $(seq 1 200); printf '\nanother short one\n'; } \ + > "$WORK/toolong.txt" +"$TOOL" put "$DISKS/editlong.img" "$WORK/toolong.txt" /long.txt >/dev/null diff --git a/Tests/manifest b/Tests/manifest index cd431ef..ea64fce 100644 --- a/Tests/manifest +++ b/Tests/manifest @@ -455,6 +455,17 @@ settleApp | CosmOS/Apps/Settle.asm | assemble | - cosmosStatus | CosmOS/Source/cosmos.asm | run | status.in | 90000000 | disks/status.img statusApp | CosmOS/Apps/Status.asm | assemble | - | - +# ---- Reading a file with long lines into the editor ---- +# +# Opened twice, because once was not enough to see it: the first open corrupted the head of +# the document and the allocator's pointer, and the second walked a list that led back into +# itself and never came back. The emulator kept running and the machine never answered. +# +# Then a file with a line past what the editor can hold, which is REFUSED rather than +# shortened - a line cut here would be written back cut, and the file damaged by having +# been looked at. +cosmosEditLong | CosmOS/Source/cosmos.asm | run | editLong.in | 200000000 | disks/editlong.img + # A failed start with NOTHING to fall back to. The mark must not become a reason to refuse # to start at all - a failure that was passing recovers here, and one that is not leaves # the machine exactly where it would have been without any of this.