Skip to content
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

Example of adding custom C functions #187

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

Conversation

guberathome
Copy link
Contributor

@guberathome guberathome commented Dec 31, 2024

One short demonstration how to compile custom code with PForth for the "unix" build.
Custom code resides in separate "custom" folder.
Setup, compilation and cleanup are handled by one shell script.

Tested with MSYS2-Cygwin, Linux, FreeBSD and NetBSD.

Edit: Changes from #184 seem to have crept in here, these still need to be fixed. Apologies.

@philburk
Copy link
Owner

philburk commented Jan 1, 2025

Changes from #184 seem to have crept in here,

there have been several recent changes. You will need to rebase to clear the merge conflicts.

@guberathome guberathome marked this pull request as draft January 1, 2025 11:36
@guberathome
Copy link
Contributor Author

guberathome commented Jan 1, 2025

there have been several recent changes. You will need to rebase to clear the merge conflicts.

Done, changes are merged.

@guberathome guberathome marked this pull request as ready for review January 1, 2025 11:54
Copy link
Owner

@philburk philburk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "custom" folder should be in an "examples" folder.

custom/cf_helpers.h Outdated Show resolved Hide resolved
platforms/unix/Makefile Outdated Show resolved Hide resolved
custom/01-be-gone/cf_demo1.c Outdated Show resolved Hide resolved
custom/01-be-gone/go-v1.sh Outdated Show resolved Hide resolved
custom/01-be-gone/go-v0.sh Outdated Show resolved Hide resolved
custom/01-be-gone/demo.fth Outdated Show resolved Hide resolved
@philburk philburk changed the title automate custom builds Example of adding custom C functions Jan 2, 2025
@guberathome
Copy link
Contributor Author

guberathome commented Jan 2, 2025

done:

  • The "custom" folder should be in an "examples" folder.
  • in cf_helpers.h: We should not encourage calling exit() or assert() in production code.
  • in Makefile: CF_ is a bit cryptic. We should use CUSTOM_SOURCES
  • in cf_demo1.c: Deleting files is a bit dangerous for a demo
    replaced BE-GONE with FILE-INFO
  • go-v1.sh: Why v0 and v1?
  • in go-v0.sh: Why overwrite the file?
    removed since we changed the Makefile

open:

  • in demo.fth: Just need one "passing"
    Sorry to disagree: passing more than just one value helps to ensure proper function.
    Also you can guess (by inductive reasoning) what F4711 and FILE-INFO are meant to do just by looking at the output (not the source):
---------------------------
show that custom code works
---------------------------
f4711( 0, 1, 10, 100, 1000 ) = ( 11 , 58 , 481 , 4711 , 47011 )
FILE-INFO:
   file{ size=5459, path='Makefile' }
   directory{ path='../../examples' }
   error{ id=2, desc='No such file or directory', path='fileNotHere' }

@guberathome guberathome requested a review from philburk January 2, 2025 09:57
@guberathome
Copy link
Contributor Author

Just need one "passing"

Please consider if these changes are relevant to be merged (discussion on #188 contains my pitch in favour).
My preference would be to contribute to this repo rather than putting example elsewhere.

I can removed the last offending passages from custom/01-be-gone/demo.fth if that settles it.

examples/custom/01-parameter-passing/cf_demo1.c Outdated Show resolved Hide resolved
examples/custom/01-parameter-passing/demo.fth Outdated Show resolved Hide resolved
examples/custom/cf_helpers.h Outdated Show resolved Hide resolved
asprintf( &result, fmtFile, info.st_size, path );
}
PUSH_DATA_STACK( (cell_t) result );
return (cell_t)strlen(result);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path is not getting freed.
Can you avoid doing the dangerous memory allocation for the C string? Maybe allocate on the stack.

Copy link
Contributor Author

@guberathome guberathome Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added FREE-C word.
Thus deallocation is done in demo.fth.

char* result;
/* MSYS/Cygwin may warn than asprintf() is not defined but compile and run just fine :-/ */
if( stat(path, &info) == -1 )
asprintf( &result, fmtErr, errno, strerror(errno), path );
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not go through the Forth output path so it will not work over a serial port.

Demonstrating custom C calling does not require calling complex host functions.
It could just be a function that modifies a string.
The user will just replace these custom functions so the simpler the better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not go through the Forth output path so it will not work over a serial port.

Just to clarify: asprintf() allocates a buffer of correct size into which it sprintf's the formatted output.
Which then gets returned.
I'm not aware of bypassing any PForth functionality here.

Demonstrating custom C calling does not require calling complex host functions. It could just be a function that modifies a string. The user will just replace these custom functions so the simpler the better.

That's what is done here.
Creating a new string (of previously unknown length) requires either hairy assumptions about maximal length or allocating a new buffer. All the fun of programming C.

@guberathome guberathome marked this pull request as draft January 7, 2025 16:43
@guberathome guberathome marked this pull request as ready for review January 7, 2025 17:56
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