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

Completion of error handling #4

Closed
elfring opened this issue Aug 11, 2014 · 11 comments
Closed

Completion of error handling #4

elfring opened this issue Aug 11, 2014 · 11 comments

Comments

@elfring
Copy link

elfring commented Aug 11, 2014

Would you like to add more error handling for return values from functions like the following?

@ocornut
Copy link
Owner

ocornut commented Aug 11, 2014

Yes. This part of the code (.ini saving/loading) was tackled in and isn't very robust. Will have a look (or feel free to submit a patch).

@elfring
Copy link
Author

elfring commented Aug 11, 2014

Are you interested to apply aspect-oriented software development?
How do you think about to encapsulate error detection and corresponding exception handling as a reusable aspect in your software?

@ocornut
Copy link
Owner

ocornut commented Aug 11, 2014

About the initial request. I have checked both LoadSettings and SaveSettings and haven't found a spot where additional meaningful error handling could be added. The return value to fread() is being used. In the unlikely case that fprintf() fails the following code can proceed happily.

Not interested in any change that would make the library less portable or carry more dependencies. Exceptions are disabled in many environment (embedded systems or game developpers typically disable exceptions in their compilers) and not a reasonable way of handling errors imho.

@elfring
Copy link
Author

elfring commented Aug 11, 2014

  • How do you think about to change the return type of functions like "LoadSettings" and "SaveSettings" to "int"?
  • Do you care if the passed data were completely written?
  • Would you like to check for failed memory allocations?

@ocornut
Copy link
Owner

ocornut commented Aug 13, 2014

  • Not needed
  • No because the .ini file only saves window size/position as a commodity
  • Possibly need to check for them more thoroughly / but effectively they don't happen or recovery is not meaningful. ImGui is engineered for adhoc debug apps.

@ocornut ocornut closed this as completed Aug 13, 2014
@elfring
Copy link
Author

elfring commented Aug 13, 2014

@ocornut
Copy link
Owner

ocornut commented Aug 13, 2014

You've been ignoring our other discussions so I am now assuming you are a bot.

@elfring
Copy link
Author

elfring commented Aug 13, 2014

There are a few different opinions around the affected error handling.
To which discussions would you like to refer to?

@ocornut
Copy link
Owner

ocornut commented Aug 13, 2014

Not using exceptions.

@elfring
Copy link
Author

elfring commented Aug 13, 2014

By which compiler parameters do you switch off the support for C++ exceptions here?
Is this an interesting implementation detail for your software portability?

@ocornut
Copy link
Owner

ocornut commented Aug 13, 2014

I don't recall the exact parameters. Many embedded and console developers don't allow exceptions because they add extra overhead which can't be afforded.

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

2 participants