Segfault on unfolding bus

[Julien Faucher]

In eeschema, create define a bus through Tools > Bus Definition.
Add a bus, named after the previously defined bus.

Hit 'D', select a wire to unfold, click and Segfault.

Please find attached a patch which seems to fix the issue (doDrawSegment is called with nullptr in argument, so I inverted the order of conditions in order to handle nullptr at the begining.

However it is not possible to unfold again unless eeschema is restarted. I haven't found why...

Best regards,
Julien FAUCHER

Application: kicad
Version: (5.1.0-372-gfcc4a84e0-dirty), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.61.0 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.4) nghttp2/1.32.1 librtmp/2.3
Platform: Linux 4.18.0-18-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 3.24
    Boost: 1.67.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.61.0
    Compiler: GCC 8.2.0 with C++ ABI 1013

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=ON
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_PYTHON3=OFF
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_WXPYTHON_PHOENIX=OFF
    KICAD_SCRIPTING_ACTION_MENU=ON
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=ON
    KICAD_USE_OCC=OFF
    KICAD_SPICE=ON

[Jon Evans (craftyjon)]

@Jeff do you have time to review this patch to SCH_DRAWING_TOOL?

[Jeff Young (jeyjey)]

@Jon and @Julien, it's not quite correct as I don't think it will place the label correctly. I'm about to submit a different fix; if you guys could let me know if it works that would be great.

[Jeff Young (jeyjey)]

https://github.com/KiCad/kicad-source-mirror/commit/3aaba79b878f34c57e5fa6e879ceafbd55d46240

[Julien Faucher (suzizecat)]

@Jeff, thanks for your quick reply. It fix the segfault issue. However there is two remaining points :
1. It stills SEGFAULT if your cursor to the other side of the bus (let say you have an horizontal bus, unfold will start something at the bottom of the bus. If you cross it so place the bus to wire connection at the top side, instant segfault. Probably for the same reason.

2. IMHO, unfolding the bus should start a wire with the associated net label at the end, following. Placing a netname, then linking it to the but to wire connection seems a little off, to me...

[John Beard (john-j-beard)]

@Jeff: how much does the layout of this tool affect your ongoing modern tool framework work?

I'm hesitant to touch any of it for fear of tripping you up.

[Jon Evans (craftyjon)]

@Jeff I just tried it and also see the same behavior Julien reports.
The original behavior was working like Julien's point 2 in #5:

1) after clicking the context menu selection of which bus member to unfold, you have a ghosted bus entry, wire(s) to the cursor, and a label at the cursor

2) after first click, the bus entry, wire(s), and label are fixed, and you drop into wire drawing mode to continue the wire

[Jeff Young (jeyjey)]

@John and @Jon,

I don't have any feelings about how this should work; I was just trying to copy the existing behaviour. To be honest I've never used a bus in eeschema.

The code in master is likely much modified by the modern tool stuff. However, I'll do another push to the eemodern branch which should at that point be pretty stable.

If you fix it there please send me a patch and I'll integrate it and then push to eemodern again. (I don't want to give up the ability to rebase that branch just yet.)

[Jeff Young (jeyjey)]

@John & @Jon,

origin/eemodern is now up-to-date. SCH_DRAWING_TOOL should be relatively stable from there.

[Jon Evans (craftyjon)]

Hi Jeff,
I looked at eemodern and it looks like bus unfolding isn't implemented in the context menu yet. Are you in the middle of that, or should someone else work on that?

[Jon Evans (craftyjon)]

Reset to Confirmed since I can still repro the crashes as noted in #5

[Jeff Young (jeyjey)]

@Jon, I haven't done anything about bus unfolding yet. Like unit selection, it requires some research into how to do menu conditionalization that's more flexible than the standard method (we can't very well have a condition for symbols-with-2-units, symbols-with-3-units, etc.)

If you want to take over the bus unfolding, then I can just copy your solution for unit selection. ;)

[Jeff Young (jeyjey)]

PS: I'm not sure how familiar you are with the modern toolkit, but in case you're not, just to explain that pervious comment a bit further:

In the legacy toolkit we build the menus just-in-time in onrightclick.

In the modern toolkit the menus are pre-built, but each item has a condition attached to it that is run just-in-time to determine whether or not to display it.

[Jeff Young (jeyjey)]

> Are you in the middle of that, or should someone else work on that?

I've nominated some guy named Jon Evans. ;)

[Jon Evans (craftyjon)]

Yes I just noticed the issue you describe in #12 last night when I was considering whether to just implement it quickly or post here :-) I can take it on

[KiCad Janitor (kicad-janitor)]

Fixed in revision cbc8faf49ba7fff8c95007398e788b9acbb4ab2f
https://git.launchpad.net/kicad/patch/?id=cbc8faf49ba7fff8c95007398e788b9acbb4ab2f