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.

Sign up for GitHub

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

Jump to bottom

Searchboxes fix crash #3

Closed
wants to merge 6 commits into from
Closed

Searchboxes fix crash #3

wants to merge 6 commits into from

Conversation

guitorri
Copy link

No description provided.

@guitorri
Fix crash due to uninitialized variable.
d4e37ca 
If a library component does not match one of the types in
SymbolWidget::createSymbol, the variables x1, x2, y1, y2 may end
up with unknown values. This can cause the `setMinimumSize` to
fail a bound size assertion crashing the application.

This patch add default values for these variables.
@guitorri
Clean commented out and no longer used code.
9c6b82c
@in3otd
Copy link
Owner

in3otd commented Sep 29, 2015

thanks for the patch; I tried to understand why this fix works/why it didn't reliably work before, but cannot get it. AFAIU, x1, x2, y1, y2, cx, cy are "initialized" here https://github.com/guitorri/qucs/blob/master/qucs/qucs-lib/symbolwidget.cpp#L423-L424 and by analyseLine() https://github.com/guitorri/qucs/blob/master/qucs/qucs-lib/symbolwidget.cpp#L460 so if the component description in the library is good these variables should contain good values, what am I missing here?

@guitorri
Copy link
Author

Just found out what you are missing. I was using a user library with components with unexpected components...

I have a library with hicumL2V2p31n devices... which are not mentined at:
https://github.com/guitorri/qucs/blob/master/qucs/qucs-lib/symbolwidget.cpp#L285-L288

Ok, now I noticed that the symbol is not shown for the very same reason...
My library happen to be loaded first and the first entry was the above one.
I guess adding the missing device is one way of solving (I will try). But still, member variables should be initialized in the constructor... no?

@in3otd
Copy link
Owner

in3otd commented Sep 29, 2015

oh, thanks! Yes, I agree the current code is not doing the Right Thing, just I did not understand why it should not work with a proper library. And surely the current code could be more robust against that.

@guitorri
qucslib: handle libraries with missing symbol
a1fe046 
Issue warning message in case a (user) library contains a component
which its symbols is not yet mapped or implemented in qucs-lib.

The component may be implemented correctly in qucs, but its symbol
is not matched (and drawn) in qucs-lib.

The painting code is somewhat duplicated.
@guitorri
qucs-lib: enable hicumL2V2p31n symbol drawing
819307e
@guitorri
qucs-lib: fix Qt3 compatibility warning
550cb6b 
Connecting from COMPAT signal (QAction::activated(int))
@guitorri
qucs-lib: silence warning about unused variable
ca49c41
@guitorri
Copy link
Author

guitorri commented Oct 2, 2015

This should be a bit more robust now.
Anyway, it seems wrong that painter code is duplicate... topic for another issue.

@guitorri guitorri closed this Oct 19, 2015
@in3otd
Copy link
Owner

in3otd commented Oct 20, 2015

oh, I realize now I forgot to merge all this 😬 ...I think GitHub notifications should have a "mark as unread" button, to help avoiding to forget things.
I saw you merged Qucs#303 and opened another PR with your fixes, thanks for taking care of this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@guitorri @in3otd