jrincayc / ucblogo-code

Berkeley Logo interpreter
https://people.eecs.berkeley.edu/~bh/logo.html
GNU General Public License v3.0
187 stars 34 forks source link

ISSUE-51: Improve UX / fix bugs related to char buffer output. #71

Closed dmalec closed 3 years ago

dmalec commented 3 years ago

Resolves #51

Summary

Details

The original bug stated that SETWRITE [WORD] causes a crash. The root cause was SETWRITE assuming a list parameter would have two items and not checking before dereferencing the second item. This crash condition also appeared in OPENWRITE [WORD].

While testing those two fixes, I noticed that, if the data written in Logo is longer than the buffer, it is not properly NULL terminated on the C side of things. The following snippet fairly reliably demonstrates the issue:

MAKE "char.buffer [buffer 10]

REPEAT 10 [
  OPENWRITE :char.buffer
  SETWRITE :char.buffer
  TYPE "0123456789ABCDEF
  CLOSE :char.buffer
  PRINT :buffer
]

Yielding:

012345678fTû
012345678fTû
012345678½fTû
012345678fTû
012345678fTû
012345678gSû
012345678gTû
012345678-gTû
012345678gTû
012345678gTû

The last issue is that allocating a buffer of 10 in Logo gives 9 writeable characters (since the 10th is reserved for a NULL terminator on the C side). I tweaked the code to allocate one extra space for the NULL character so that this detail isn't exposed on the Logo side of things.

Test Environments

OSX Catalina 10.15.7 w/ wxWidgets 3.0.5 Ubuntu Bionic (18.04.5) w/ wxWidgets 3.0.5 Windows 10 Home (1909) w/ wxWidgets 3.0.5

jrincayc commented 3 years ago

@dmalec Thanks for the fix.