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

[Coverity] Wrap main() with try-catch block (#1839950) #2914

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

dkjung
Copy link
Collaborator

@dkjung dkjung commented Feb 5, 2025

To address coverity issue #1839950, the main function is wrapped with try-catch block.

Self evaluation:

  1. Build test: [*]Passed [ ]Failed [ ]Skipped
  2. Run test: [*]Passed [ ]Failed [ ]Skipped

@dkjung dkjung changed the title Wrap main func with try-catch blocks [Coverity] Wrap main func with try-catch blocks Feb 5, 2025
@dkjung dkjung force-pushed the feature/coverity-1839950 branch from f2b6c15 to 8eb53c4 Compare February 5, 2025 11:11
@dkjung dkjung changed the title [Coverity] Wrap main func with try-catch blocks [Coverity] Wrap main() with try-catch blocks Feb 5, 2025
@dkjung dkjung changed the title [Coverity] Wrap main() with try-catch blocks [Coverity] Wrap main() with try-catch block Feb 5, 2025
@dkjung dkjung changed the title [Coverity] Wrap main() with try-catch block [Coverity] Wrap main() with try-catch block (#1839950) Feb 5, 2025
@dkjung dkjung marked this pull request as ready for review February 6, 2025 07:10
Copy link
Collaborator

@jijoongmoon jijoongmoon left a comment

Choose a reason for hiding this comment

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

Do we really have to wrap the main with try and catch?

@myungjoo
Copy link
Member

myungjoo commented Feb 9, 2025

Anyway.. do you need nested try here?

try {
} catch(A) { ... }
catch(B) { ... }
catch(C) { ... }

is generally better than

try {
   try {
   } catch (B) { ... }
   try {
   } catch (A) { ... }
} catch(C) { ...}

@dkjung
Copy link
Collaborator Author

dkjung commented Feb 10, 2025

@jijoongmoon @myungjoo

The Coverity issue didn't specify the line where the unhandled exception (std::bad_array_new exception) could possibly be thrown, so I covered the whole the main function with a try-catch block.

Catch a `std::bad_array_new_length` that can be risen when array
indexing

Signed-off-by: Daekyoung Jung <[email protected]>
@dkjung dkjung force-pushed the feature/coverity-1839950 branch from 8eb53c4 to 32bdc74 Compare February 10, 2025 04:42
@dkjung dkjung marked this pull request as draft February 10, 2025 04:42
@dkjung dkjung marked this pull request as ready for review February 10, 2025 06:32
@dkjung
Copy link
Collaborator Author

dkjung commented Feb 10, 2025

Anyway.. do you need nested try here?

try {
} catch(A) { ... }
catch(B) { ... }
catch(C) { ... }

is generally better than

try {
   try {
   } catch (B) { ... }
   try {
   } catch (A) { ... }
} catch(C) { ...}

Now I fixed it. I missed the exception-throwing line that the issue had reported.

Copy link
Member

@DonghakPark DonghakPark left a comment

Choose a reason for hiding this comment

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

LGTM!

@jijoongmoon jijoongmoon merged commit e0ec5e1 into nnstreamer:main Feb 11, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants