debugging: fix openocd closing when pressing Ctrl+C in GDB, fix #3427#3619
Conversation
dist/tools/openocd/openocd.sh
Outdated
There was a problem hiding this comment.
the +2 feels kind of arbitrary. Is it guaranteed to be the same every time on every system?
There was a problem hiding this comment.
This bugs me too, but I didn't find a better solution using standard POSIX tools. That's why this needs to be tested on many systems. Any suggestions?
Am 12. August 2015 19:17:40 MESZ, schrieb Joakim Gebart [email protected]:
@@ -202,7 +202,7 @@ do_debug() {
-c 'halt'
-l /dev/null" &
# save PID for terminating the server afterwards
- OCD_PID=$?
- OCD_PID=$(($!+2))
the +2 feels kind of arbitrary. Is it guaranteed to be the same every
time on every system?
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/3619/files#r36886983
There was a problem hiding this comment.
an idea:
use the $$ variable in combination with some variation of sh -c "echo \$\$; xxx" & and some output redirection
Test with: setsid sh -c "echo child: \$\$; sleep 2" &; echo "setsid: $\!"; echo "shell: $$"
|
@gebart |
dist/tools/openocd/openocd.sh
Outdated
There was a problem hiding this comment.
here's an example of using mktemp from another script I wrote:
TMPFILE=
trap '[ -n "${TMPFILE}"] && rm -f "${TMPFILE}"' EXIT
TMPFILE=$(mktemp "${TARGET_DIR}/my_temp_file.XXXXXXXXXX")
wget "${SOURCE_URI}" -O "${TMPFILE}" && ( chmod a+r "${TMPFILE}"; mv -b -f "${TMPFILE}" "${TARGET_DIR}/the_real_target.zip" )
trap - EXIT|
Changes look reasonable, but please make them conditional to Linux. Didn't hear about the |
|
@thomaseichinger Could you please test again on Mac and FreeBSD? Hopefully I used |
3545021 to
ee19baa
Compare
|
Seems I was testing against the wrong OpenOCD version. This problem does exist on OS X too. To get this in quickly, could you change to test for presence of There does exist a |
|
@thomaseichinger done |
|
Works for me. Could an other Linux user verify? But I think you can proceed with squashing. |
774222d to
8258707
Compare
dist/tools/openocd/openocd.sh
Outdated
There was a problem hiding this comment.
Actually, it makes more sense to check whether the SETSID variable is null. (sorry for commenting in the commit before)
[ -z "${SETSID}" ] && SETSID=$(which setsid)edit: fixed mixup between -n, -z
There was a problem hiding this comment.
[ -n "${SETSID}" ] && SETSID=$(which setsid)I don't get this. If $SETSID is not empty, then set it to the output of which setsid? Why should $SETSID be set in the first place?
There was a problem hiding this comment.
Anyway, where is $SETSID supposed to be set?
There was a problem hiding this comment.
The purpose was to allow overriding it from the environment in the case
where you have the osx workaround installed somewhere outside of the normal
path
On Aug 27, 2015 3:23 PM, "Daniel Krebs" [email protected] wrote:
In dist/tools/openocd/openocd.sh
#3619 (comment):@@ -191,8 +191,16 @@ do_debug() {
test_elffile
test_ports
test_tui
setsid is needed so that Ctrl+C in GDB doesn't kill OpenOCD
- [ -n "$(which setsid)" ] && SETSID=setsid
Anyway, where is $SETSID supposed to be set?
—
Reply to this email directly or view it on GitHub
https://github.com/RIOT-OS/RIOT/pull/3619/files#r38093810.
There was a problem hiding this comment.
I see. And I realized that the checking isn't needed in my solution, because SETSID="$(which setsid)" already does it.
|
Wanna squash again? |
58689ed to
78b98b3
Compare
|
@OlegHahm |
|
I never had this problem, but doesn't seem to break anything. @gebart, @authmillenon, can you verify? |
|
It seems to be related to recent OpenOCD versions, maybe >= 0.9.0 |
|
I won't be able to verify until the end of next week at the earliest.
|
Hopefully fixes #3427.
Please test this on as many system as possible, especially on Mac OS. The problem seemed to be, that openocd stays in the same process group when forking it (
openocd .... &). Now in GDB when you press Ctrl+c theSIGINTsignal will be sent to the parent shell, that redirects it to GDB as well as to openocd.Solution: detach openocd from process group.
The script also used the wrong environment variable to grab the openocd PID accroding to this.