Re: [hatari-devel] Recent Hatari change problems

[ Thread Index | Date Index | More Archives ]


On 04/11/2018 11:08 AM, Thomas Huth wrote:
On 11.04.2018 01:23, Eero Tamminen wrote:
You have been doing some changes to the command line options,
e.g. changing --xbios-intercept from toggle to just unconditionally
enabling it.

Note that all command line options are also options for the debugger.
Things that you toggle on from command line, can be toggled off from
debugger.  The earlier functionality was:
$ ./src/hatari --bios-intercept
XBios 11/20/255 Hatari versions enabled: Dbmsg(), Scrdmp(),
You have entered debug mode. Type c to continue emulation, h for help.
setopt --bios-intercept
XBios 11/20/255 Hatari versions disabled.

Ah, ok, now it makes more sense that this option was written in such a
cumbersome way ;-)

While I appreciate you adding more automated testing to Hatari...

Before making random changes to Hatari's public interface,
*please* discuss them on the mailing list.

E.g. changing option output to happen only with the lowest "debug"
level is just broken.  It means that user doesn't get any feedback
when he changes something in debugger.  Please fix that.

In some cases it's pretty important information:
> setopt --disasm 0xf
Changed CPU disassembly output flags from 15 to 0.
> setopt --disasm 15
Changed CPU disassembly output flags from 0 to 15.

With your change, user wouldn't know that given option doesn't accept
hex values for the bitmask, but interprets them as decimals. I.e.
options output should be at least the same as Hatari default
log level.

(Using stderr in your new Opt_DbgPrintf() function is fine as it
matches what debugger uses for user interaction output.)

Anyway, "toggling" is a bad idea here, since the users first have to
change the option to know in which state it was.

It's off by default and doesn't have config option, because having
it enabled by default would be a security issue.  Emulated programs
can use it e.g. overwrite any files that Hatari can access.

Note that the feature itself could have been used to disable it:
$ hatari --bios-intercept
<trusted emulated code using XBios(255)>
XBios(255, "hatari-option --bios-intercept") call to turn it off
<untrusted emulated code>

I've now changed it to a proper "boolean" option
("--bios-intercept on|off") which should be
more straight forward and fix this problem.

That breaks any programs that were using the above XBios() call.
If such changes are deemed necessary, they must be listed in
the release notes (maybe under "API changes" heading).

When updating command line options, please update both the Hatari
HTML manual and manual page, not just one of them.

Btw. Why the commit to make XBios test "more reliable", changes Hatari
to be run with "--cpuclock 32" instead of using "--fast-forward on"?

FYI: Some other options that can be used to make emulation faster
in addition to "--fast-forward yes" and "--sound off":
* "--borders off" when overscan effects don't need to be visible
* "--spec512 0" when Spec512 effects don't need to be visible
* "--zoom 1" to lessen Hatari screen conversion overhead
* "--disable-video yes" when display output isn't needed
* "--dsp off" when tests don't use DSP (for Falcon)
* "--fastfdc yes" when tests don't use floppy
* "--fast-boot yes" when TOS memory detection isn't needed
* "--timer-d yes" when tests don't depend on timer-D interval

(Last 4 can have some effect on emulated code, first ones don't
affect emulation.)

I would also recommend "--mousewarp off" to avoid Videl code
"randomly" moving mouse on screen when emulation is booted.

As to converting dbmsg.c tester to assembly... While one reason
for the tests is making sure that Hatari functionality doesn't
regress, the other reason is having example code for the users.

IMHO the old C version was  more readable than the assembly
one.  This API is so rarely used, that I don't think it matters
though. :-)

	- Eero

Mail converted by MHonArc 2.6.19+