Skip to content

Ptchsize and MacOS fix #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

boeckmann
Copy link
Contributor

This merge request modifies ptchsize to require a command line argument for the compiler used to compile freecom. It also modifies the build.sh and build.bat scripts accordingly.

The additional heap is reduced to 6k again, as it should be sufficient?!?

build.sh is fixed to run on MacOS.

@diekleineHexe
Copy link

I don't think making this compiler dependent is a smart idea,
each command.com needs a minimum of heap. and no compiler on this planet can know
how much is needed.
with XMS_SWAP active, that's irrelevant
without XMS_SWAP, the heap is frozen as soon as KEYB is loaded.
later commands my use more heap then is currently allocated.
command minimum heap really should be allocated through EXE header.

but anyway (I don't care much about GCC)

if (compiler != COMPILER_GCC) {
if(fseek(freecom, ival.heapPos, SEEK_SET) != 0) {
printf("Failed to seek to heap size offset in %s\n", argv[1]);
return 42;
}
assert(sizeof(tosize) == 2);
if(fwrite(&tosize, sizeof(tosize), 1, freecom) != 1) {
printf("Failed to patch heap size into FreeCOM executable.\n"
"File most probably corrupted now: %s\n", argv[1]);
return 75;
}
}

silently doing nothing when compiler is GCC isn't smart (as the last version did). please do at least

...
else
printf("while instructed to do something, I ignore you because GCC doesn't need this\n");

@boeckmann
Copy link
Contributor Author

I don't think making this compiler dependent is a smart idea.

Me too. But I am currently not sure what the side effects of changing it are. So for the moment this is a minimal patch to make it work again, until I or someone else has the time to deal with it in the proper way. I will add the message you suggested.

Related: #169

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants