Repository navigation
ReadArgs: read the command line from the Input() buffer, as AmigaDOS does - #205
Merged
Merged
Conversation
sidick
reviewed
Oct 24, 2026
sidick
left a comment
Owner
There was a problem hiding this comment.
In run_command_handler, copying param_len bytes directly into a host Vec lets a guest supply an extremely large length and trigger a huge allocation/read loop, potentially exhausting the host. Please validate the length against the available guest memory (and reject lengths that cannot be safely copied) before allocating.
sidick
requested changes
Oct 24, 2026
…does RunCommand stuffs the arguments into the buffer of Input() and restores it when the command returns, and ReadArgs, FGetC, FGets and FRead read them from there; ReadArgs no longer parses the guest's A0 buffer, which C startup code edits (vbcc's writes a NUL over the '\n'). RunCommand refuses an argument range past guest memory. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
codewiz
force-pushed
the
readargs-own-copy
branch
from
October 24, 2026 07:11
c3cfddb to
cd23093
Compare
Contributor
Author
|
PTAL |
sidick
approved these changes
Oct 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ReadArgs(NULL)used to parse the guest's A0/D0 command-line buffer. That buffer belongs to the program, and C startup code edits it in place while splitting it into argv: vbcc's writes a NUL over the trailing'\n'. A vbcc-built program run asprog LISTtherefore gotLIST\0, andprog ITER 1failed with "Bad number". The same programs work under vamos and on AmigaOS.On AmigaOS,
ReadArgsnever looks at that buffer.RunCommandputs the arguments into the buffer of the process'sInput()filehandle, andReadArgsreads them from there:RunCommandautodoc: "RunCommand also takes care of setting up the current input filehandle in such a way that ReadArgs() can be used in the program, and restores the state of the buffering before returning." It also says the argument string must end with a newline "to work properly with ReadArgs()".ReadArgsautodoc: "Normally gets the arguments by reading buffered IO from Input(), but also can be made to parse a string."ReadItemautodoc: with noCSourceit "uses FGetC(Input())".rom/dos/runcommand.ccallsvbuf_inject(Input(), argptr, argsize)before running the command and flushesInput()after it returns;rom/dos/createnewproc.cdoes the same forNP_Arguments;rom/dos/readargs.creads fromInput()when noRDA_Sourceis given: "Take arguments from input stream. They were injected there by either runcommand.c or createnewproc.c".This PR does the same:
DosState::cmdline/cmdline_posbecomeinput_buf/input_pos, the bytes in theInput()buffer.Runtime::newputs the process's command line there.RunCommandputs the child's arguments there. When the child returns, it restores the caller's buffer and position, as the autodoc says. sidick's review: it first checks thatargptr + argsizelies inside guest memory. If it doesn't, it returns -1 withERROR_LINE_TOO_LONG, before allocating or copying anything.ReadArgs(NULL)reads frominput_buf. When that's used up, it sees an empty line rather than waiting on host stdin, as AROS does for an interactiveInput()with an empty buffer.FGetC,FGetsandFReadon the currentInput()return the remaining buffered bytes before host stdin, through a newDosState::read_buffered_byte. PlainReadbypasses the buffer, as on AmigaOS, so a program reading stdin withReadnever sees its arguments.Tests, all new or updated:
the_command_line_is_also_in_the_input_buffer_for_readargs;buffered_reads_of_input_return_the_command_line_first;run_command_rejects_arguments_past_guest_memory;run_command_resets_readargs_cursor_for_the_child_instead_of_inheriting_the_callers, now oninput_pos.cargo test --workspaceotherwise has the same two failures as main (pending_host_break_folds_into_check_signal,pending_host_break_folds_into_wait). Checked with BenchWork's vbcc and gcc builds:LIST,REPEAT 1 x25519,-handREPEAT=0all behave as under vamos. Rebased on main, which now has #204.🤖 Generated with Claude Code