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

[utils] coverity: ignore uncaught 'fmt::FormatError' exception #12399

Merged
merged 1 commit into from Jul 6, 2017

Conversation

Rechi
Copy link
Member

@Rechi Rechi commented Jun 30, 2017

Description

Disable an fmt::FormatError uncaught exception issue at coverity.

Motivation and Context

The windows coverity project currently shows 63 warnings because of the exception thrown in fmt.
If fmt throws an exception it should result in a crash as in that case the format string is wrong.

How Has This Been Tested?

Next coverity after this has been merged will tell if it does what I expect.

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@Rechi Rechi added the v18 Leia label Jun 30, 2017
@Rechi Rechi added this to the L 18.0-alpha1 milestone Jun 30, 2017
@Rechi Rechi changed the title [utils] coverity: ignore uncaught '' exception [utils] coverity: ignore uncaught 'fmt::FormatError' exception Jun 30, 2017
@garbear
Copy link
Member

garbear commented Jun 30, 2017

Instead of hard-crashing, what about this as a workaround? #11915

When the fmt conversion happened, one of the strings had a wrong format type and it took a long time to track it down. Adding this log message instead of crashing would be helpful in that case.

@FernetMenta
Copy link
Contributor

A wrong format is not a runtime error. Having the app crash is much better than hiding the error.

@Rechi Rechi merged commit 6dbfa7c into xbmc:master Jul 6, 2017
@Rechi Rechi deleted the coverityFmtException branch July 6, 2017 13:06
@garbear
Copy link
Member

garbear commented Jul 6, 2017

A wrong format is not a runtime error. Having the app crash is much better than hiding the error.

But crash without error logging? And now a translator can maliciously or accidentally crash Kodi when a new language is installed.

@AlwinEsch
Copy link
Member

This is also a point where I must say that a "try" is helpful, after removing from the addons it makes me almost impossible to find the source caused.

Why not the "try" log the error and "throw" again?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants