Re: [hatari-devel] More memory options

[Eero Tamminen <oak@xxxxxxxxxxxxxx>]

Hi,

Sorry, reading other patches, I noticed that the validate function
provides backwards compatibility, if value is <= 14, it's interpreted
as MBs instead of KBs, which is used both for config & options.

So it's fine, sorry again for too hasty comment.


Other comments:
* You added "mem" trace option, but no tracepoints using it?
* TT-RAM change is IMHO superfluous and it changes meaning of memory
  shapshot value, so memory snapshots with TT-RAM aren't anymore
  compatible.  What if you instead would add _MB extension to
  TTRamSize variable?


	- Eero

On 12/17/2016 11:13 AM, Eero Tamminen wrote:
> On 12/14/2016 08:03 PM, Nicolas Pomarède wrote:
> > as part of some ongoing changes I'm working on, I added some new options
> > to allow RAM size using 128 KB, 256 KB or 512 KB in a more flexible way
> > than what was used before by specifying only a number of MB.
> > So, it's now possible to configure Hatari to use 256 KB or 2.5 MB for
> > example.
> >
> > I also updated/renamed nMemorySize and nTTRamSize to STRamSize_KB and
> > TTRamSize_KB for consistency and readability (only in the code, the
> > config file still uses nMemorySize and nTTRamSize, although this could
> > be changed too).
>
> As it uses different units, it should use different config file options.
> Working defaults are better than earlier saved values that are nowadays
> bogus.
>
>
> Changing meaning of command line arguments is maybe even worse.
> People don't know why things don't anymore work with their old scripts
> when you do that.
>
> You should add another option for KBs and either return error for
> the old option, or (preferable) just multiply the given value by
> 1024.
>
> (One of the scripts is the Hatari TOS-boot tester which is used before
> releases to validate that Hatari still works with all TOS versions.)
>
>
> > Both variables are now in KB instead of MB to allow sizes not multiple
> > of MB.