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

BUG: Fix crash in ctkDICOMQueryRetrieveWidget #1057

Merged
merged 1 commit into from
Dec 6, 2022

Conversation

lassoan
Copy link
Member

@lassoan lassoan commented Dec 6, 2022

After multiple queries with different criteria, the query/retrieve widget crashed due to null-pointer dereference. It was not clear what led to the error, but the null-pointer check in this commit prevents the application from crashing, which is already an improvement.

@lassoan lassoan requested a review from pieper December 6, 2022 00:49
@lassoan lassoan self-assigned this Dec 6, 2022
@jcfr
Copy link
Member

jcfr commented Dec 6, 2022

Instead of the crash, should the user expect to the warning message ?

May be worth adding a comment in the code with a link to the original issue or report documenting the crash.

@lassoan
Copy link
Member Author

lassoan commented Dec 6, 2022

We'll use query-retrieve in a project, so it will get some more testing and improvements, but right now I have no more information than what is in the commit comment. I could add a ticket for the crash but this PR fixes it, so it would not help much. If added an issue for query/retrieve failing then it may be useful, but only if I had more time to do some investigation, maybe reproduce the issue - but I'm not sure when I'm going to do that. I would prefer to just integrate this as is and then later we'll make query-retrieve more robust.

Copy link
Member

@pieper pieper left a comment

Choose a reason for hiding this comment

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

This seems like a reasonable workaround for now. I don't know what the underlying cause is.

After multiple queries with different criteria, the query/retrieve widget crashed due to null-pointer dereference.
It was not clear what led to the error, but the null-pointer check in this commit prevents the application from crashing, which is already an improvement.
@pieper
Copy link
Member

pieper commented Dec 6, 2022

I think we were both clicking at the same time @jcfr - sorry if this led to a bad state.

@jcfr
Copy link
Member

jcfr commented Dec 6, 2022

Thanks for the review.

Should now be good for integration 🚀 I will let you go ahead and merge 👌

@pieper pieper merged commit b23b52d into commontk:master Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants