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

Replace std::abort() with std::runtime_error #1245

Merged
merged 7 commits into from
Jun 14, 2016
Merged

Conversation

norihiro-w
Copy link
Collaborator

@norihiro-w norihiro-w commented Jun 10, 2016

as issued in #1165. I added ERRMSG macro (see 74bd882) in StringTools.h so that one can write as

throw std::runtime_error(ERRMSG("Some error with type %d", -1));

which generates an error message "Some error with type -1 at (file name), line (line no)".

@chleh
Copy link
Collaborator

chleh commented Jun 13, 2016

Wouldn't it be better to introduce a macro OGS_ERROR(...) or OGS_FATAL(...) instead of always having to write throw std::runtime_error(ERRMSG(...))?

#ifdef MSVC
vsnprintf_s(buffer, size_of_buffer, _TRUNCATE, format_str.data(), args);
#else
vsnprintf(buffer, size_of_buffer, format_str, args );
Copy link
Collaborator

@chleh chleh Jun 13, 2016

Choose a reason for hiding this comment

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

Error messages of more than 256 characters are pretty long, but I think there should be a check if the buffer was long enough, and if not a bigger buffer should be allocated dynamically (cf. "Notes" in http://en.cppreference.com/w/cpp/io/c/fprintf). ✅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will check it

@chleh
Copy link
Collaborator

chleh commented Jun 13, 2016

Will the error message of the exception always be printed? (I.e., in debug and release builds?)
Or do we need a dedicated try/catch block in the main function for that?

@chleh
Copy link
Collaborator

chleh commented Jun 13, 2016

After the questions are answered 👍

@norihiro-w
Copy link
Collaborator Author

  • Having OGS_ERROR macro is also fine, though explicitly writing std::runtime_error makes it clear what is really happening. I would like to hear how others think about it.
  • I will add a try/catch block in the main function. IMO those error messages should be printed also in release mode.

@endJunction
Copy link
Member

At first I agreed with Christoph having a single macro like OGS_FATAL. This could be also replaced with something different than throw. On the other hand there are more exception types than the runtime error; having a single macro takes the opportunity to have different error handling...

@chleh
Copy link
Collaborator

chleh commented Jun 13, 2016

But catching exceptions occasionally is discouraged, AFAIK...

@norihiro-w
Copy link
Collaborator Author

how about a macro OGS_RUNTIME_ERROR? In the future, someone could add OGS_LOGICAL_ERROR or something else.

@chleh
Copy link
Collaborator

chleh commented Jun 14, 2016

OGS_RUNTIME_ERROR is OK.
But is there the chance that anyone of us will ever make a distinction between different kinds of exceptions thrown?

@norihiro-w
Copy link
Collaborator Author

no idea yet

@norihiro-w
Copy link
Collaborator Author

or shall we use OGS_FATAL now and rename it later if someone wants to use different exceptions?

@norihiro-w
Copy link
Collaborator Author

updated. please check each commit

@chleh
Copy link
Collaborator

chleh commented Jun 14, 2016

perfect. 👍

@ogsbot
Copy link
Member

ogsbot commented Jun 19, 2020

OpenGeoSys development has been moved to GitLab.

See this pull request on GitLab.

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.

5 participants