Skip to content

Bug #3611 Initializing static cv::Mat with cv::Mat::zeros causes segmentation fault fixed.#2558

Merged
opencv-pushbot merged 1 commit intoopencv:2.4from
asmorkalov:ocv_matop_init_fix
Jul 8, 2014
Merged

Bug #3611 Initializing static cv::Mat with cv::Mat::zeros causes segmentation fault fixed.#2558
opencv-pushbot merged 1 commit intoopencv:2.4from
asmorkalov:ocv_matop_init_fix

Conversation

@asmorkalov
Copy link
Copy Markdown
Contributor

No description provided.

@asmorkalov
Copy link
Copy Markdown
Contributor Author

@SpecLad please look at it. I think it is ok to merge the fix after release.

@SpecLad
Copy link
Copy Markdown

SpecLad commented Apr 2, 2014

You're trading initialization order problems for thread safety problems. I'm not so sure this is a good tradeoff.

There are other global MatOp objects besides g_MatOp_Initializer. Wouldn't they need to be fixed in the same way?

@ghost ghost assigned SpecLad Apr 2, 2014
@ghost
Copy link
Copy Markdown

ghost commented Apr 2, 2014

Roman,

I have assigned the PR to you.

@ghost ghost closed this Apr 2, 2014
@ghost ghost reopened this Apr 2, 2014
@vpisarev
Copy link
Copy Markdown
Contributor

vpisarev commented Apr 8, 2014

@SpecLad, the patch is harmless. The MatOp_Initializer class does not have any mutable members (mutable in generic, not C++ sense), so there are no thead-safety problems. I think, we should merge the patch. If something else needs to be done with other MatOp objects, that can be done in further PR. I do not see any problem with solving the problems step by step.

@Daniil-Osokin
Copy link
Copy Markdown

@SpecLad Does this make sense?

@SpecLad
Copy link
Copy Markdown

SpecLad commented Jul 8, 2014

Yeah, I guess it's okay. :shipit:

@Daniil-Osokin
Copy link
Copy Markdown

Wow, so fast, thanks!

@SpecLad
Copy link
Copy Markdown

SpecLad commented Jul 8, 2014

Yeah, only took, like, what, 3 months? 😃

@opencv-pushbot opencv-pushbot merged commit 0c30b18 into opencv:2.4 Jul 8, 2014
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