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

Return some error codes #21

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

Conversation

sicklittlemonkey
Copy link
Contributor

A starter for #20.

I've only tested ADDFILE.

- AddFile in ADDFILE
- AddFile in REPLACEFILE
- Set/ClearFileHighBit first error (untested!)
- In/OutdentFile first error (untested!)
@mach-kernel
Copy link
Owner

Awesome! This was super annoying while using in build scripts that should fail early but no way to tell.

@sicklittlemonkey
Copy link
Contributor Author

The other code paths all have good error-handling code, but don't return int.

It should be easy to change.

Returns any error from ExtractOneFile.
Better to pass up the error from called functions.
@sicklittlemonkey
Copy link
Contributor Author

Ok, I did EXTRACTFILE in 5 minutes - but not tested! ; - )

Look at the existing error handling in AddFile.

If there's an error:

  • Update the current image, i.e. current_image->nb_add_error++ or nb_extract_error++;
  • Free memory if it won't be used again (e.g. you're returning now) e.g. mem_free_file(current_file);
  • In general return(1), though if there's an error variable it's better to return(error); IMO.

The other ones get a bit more involved because there are loops etc. (Damn I wish this was in C++.)

I'll try to do a more complex one later and see how hard it gets.

Cheers,
Nick.

@mach-kernel
Copy link
Owner

I'll do some testing this evening 👍

(Damn I wish this was in C++.)

I was playing around with the idea of using Go with Cgo to write a niftier CLI tool while using all of the existing ProDOS code as a library.

@sicklittlemonkey
Copy link
Contributor Author

Ok, I have no idea what's going in my fork anymore. No time for this nonsense. ; - )

@mach-kernel mach-kernel closed this Jan 2, 2020
@sicklittlemonkey
Copy link
Contributor Author

Oh, what happened to this one? Sorry, Git was pissing me off and I haven't had time to get back to it.

@mach-kernel mach-kernel reopened this Jan 3, 2020
@mach-kernel
Copy link
Owner

Hi there! I reopened it, thought it was abandoned because of the merge conflicts. I think this feature is necessary if people want to use this in any kind of script. Do we want to just do 0/1 or come up with more granular code semantics? I'm happy to help rebase + do stuff for this branch if you would welcome that! 😸

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