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

INCBIN/BIN bugfix, code cleanup #27

Closed
koitsu opened this issue Aug 12, 2019 · 2 comments
Closed

INCBIN/BIN bugfix, code cleanup #27

koitsu opened this issue Aug 12, 2019 · 2 comments

Comments

@koitsu
Copy link
Contributor

koitsu commented Aug 12, 2019

Just wanted to note this here: you may find some, most, or even all of my commits here useful: https://github.com/koitsu/asm6f/commits/master

An executive summary would be the following:

  • Bugfix: INCBIN/BIN fix for unquoted filenames using size/offset args
  • Other: Document that INCBIN/BIN and INCLUDE/INCSRC support quoted filenames
  • Other: Document how to build asm6f and its pre-requisites (ex. GNU make)
  • Other: Revamped Makefile for proper dependencies, effectively add clang support, hopefully better Windows support (untested), enforcing more extreme warnings when build=debug (justifies most of the cleanup commits), and treating warnings as errors
  • Code safety: use snprintf() instead of sprintf()
  • Code safety: cease use of vfprintf() and friends (valgrind et al does not like it)
  • Cleanup: removing trailing tabs/spaces, as well as blank lines with such
  • Cleanup: add many missing function prototypes
  • Cleanup: use var->member not (*var).member, constify things, make my_malloc() comply with real-world malloc()
  • Cleanup: don't cast findlabel()'s return value to ptrdiff_t
  • Enhancement: make INCBIN/BIN show an error if the calculated fread() size doesn't match what was read off disk (this usually means disk I/O error when reading). This needs further testing, but pretty sure I tested it well...

At bare minimum, the INCBIN/BIN fix from 40706e3 should be brought in. I would strongly suggest parts of the Makefile as well, particularly proper use of make variables for dependencies.

The warning-related stuff took me several hours to actually clean up/work out. When I started, I actually had to use -ferror-limit=0 due to the code having almost 200 warnings once more extreme warning flags were added.

Anyway, I'd love for these to be brought in, simply because we have enough forks already... :-)

All of this needs very thorough testing, i.e. on a big/hefty project. There are some things I changed (example) that make me a little bit wary.

@koitsu
Copy link
Contributor Author

koitsu commented Jan 5, 2023

Some of these have been addressed:

However, the big patch (for cleaning up a lot of the code) as well as changing the Makefile have yet to be put into their own PRs. Hopefully I'll get to that this month. Have the code already done, just need to PR it.

@koitsu
Copy link
Contributor Author

koitsu commented Jan 22, 2023

Going to close this ticket since I'm just going to do all of this in PRs (ex. PR #36 and PR #37 with more to come).

@koitsu koitsu closed this as completed Jan 22, 2023
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

No branches or pull requests

1 participant