search

dasm-assembler / dasm

Macro assembler with support for several 8-bit microprocessors
https://dasm-assembler.github.io/
GNU General Public License v2.0
215 stars 40 forks source link

bugfix to segmentation fault and support for last missing opcodes #99

Closed thomas374b closed 4 years ago

thomas374b commented 4 years ago

Three minor issues

A) bugfix to segmentation fault (issue #98), avoid calling NULL pointer by exiting with proper error code and -message, two .ref files removed and .fail added to revert the sense/result of the testcase

B) support for missing opcodes for 68hc908 added Those special not-yet supported opcodes cant fit well into the current implemention scheme of dasm but I've found a way how they can be supported by using the address-mode extension feature. The result doesn't look clean but better this way than nothing. The other was just missing

C) I did a review of all opcodes for the 68hc908 processor and added three files to support documentation. I used gnumeric to create the table and later exported it to .tex and .html format. Only one of the 68hc908_opcode_summary.* files needs to be kept.

dionoid commented 4 years ago

Hi Thomas, when you create a pull request from your master, it means that every commit you do to your master branch after you created your pull request will automatically be added to the pull request. That's probably not what you intended to do.

The normal flow for creating a pull request is to create a branch in your forked repo, in which you commit your changes. Then you can create a pull request from this branch, which allows the dasm maintainers to review and merge your changes. Please see https://opensource.com/article/19/7/create-pull-request-github

thomas374b commented 4 years ago

I wanted these files to be included in the pull request. However if it creates extra fuzz please ignore the commits following 248d936

I'm still working on this coloring in 68hc908_opcode_summary.tex

To have a man-page is a must on any unix system. I've had this topic in my queue too. Of course the initial version is a little bit thin but not everything from dasm.pdf needs to be included there. But it should go beyond only describing commandline arguments.

I can contribute also a set of Makefiles to create a debian installation package.

dionoid commented 4 years ago

I cherry-picked these commits back into the master DASM branch:

Thanks again for these changes!

As a suggestion: it would be great if for future pull requests, you create separate branches in your fork of DASM for each fix. This way, you can create an isolated pull request, while you can continu to work on other stuff in other branches. It also makes it easy for others to review fixes and merge them back into DASM, without having to cherry-pick commits :-)

Btw: the commit named "processor 68hc908 is also supported" is based on an open PR (from jnahmias), so I couldn't process that one.

thomas374b commented 4 years ago

OK, I close this pull request since I want continue commiting to my master. The important stuff has been cherry-picked. The tex stuff goes into a next pull request.