-
-
Notifications
You must be signed in to change notification settings - Fork 781
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
pc-hosted semihosting #665
Conversation
ret = rename(fnam1, fnam2); | ||
free(fnam1); | ||
free(fnam2); | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these error handling conditions after successful malloc
's fail to free
the buffers before breaking. It looks like this is also an issue in some of the other commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right.
src/target/cortexm.c
Outdated
if (target_check_error(t)) break; | ||
ret = write(params[0] - 1, buf, buf_len); | ||
free(buf); | ||
//fflush(stdout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you wish to delete?
Oh I just meant delete the commented out code
…On Sun, May 24, 2020 at 11:59 AM Koen De Vleeschauwer < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/target/cortexm.c
<#665 (comment)>
:
> + break;
+ }
+
+ case SYS_WRITE: { /* write */
+ ret = -1;
+ target_addr buf_taddr = params[1];
+ uint32_t buf_len = params[2];
+ if (buf_taddr == TARGET_NULL) break;
+ if (buf_len == 0) {ret = 0; break;}
+ uint8_t *buf = malloc(buf_len);
+ if (buf == NULL) break;
+ target_mem_read(t, buf, buf_taddr, buf_len);
+ if (target_check_error(t)) break;
+ ret = write(params[0] - 1, buf, buf_len);
+ free(buf);
+ //fflush(stdout);
What do you wish to delete?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#665 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC5YNWUNRLOH4B7AH344ILTRTFVCJANCNFSM4NDQCDHA>
.
|
fixed, check again. |
|
On Mon, 25 May 2020 14:57:20 -0700 Tyler Holmes ***@***.***> wrote:
`malloc`/`free` fix looks good! Can't really review the rest with
much authority though, as I'm pretty new to all of this code.
OK. It's the semihosting syscalls for the pc-based version of bmp.
|
As stated on discord, please state explicit when you want me to merge. |
On Tue, 26 May 2020 05:24:56 -0700 UweBonnes ***@***.***> wrote:
As stated on discord, please state explicit when you want me to merge.
OK. Please merge this PR.
regards,
koen
|
It is a good idea to rebase to master and run git log --check and git rebase --whitespace=fix master if problems show up. |
ok. done. |
Can you please check that my multi branch has rebase your changes in the right way? |
Source looks good. On Raspberry Raspbian "Buster": Recognizes jlink, cmsis-dap, bmp. |
As requested, semihosting for "blackmagic_hosted".
PC-hosted semihosting does keyboard, file and screen i/o on the system where blackmagic_hosted runs, using linux system calls, while semihosting in the probe does keyboard, file and screen i/o on the system where gdb runs, using gdb file i/o calls.
The pc-hosted semihosting was tested with the sketches from the Arduino STM32duino-semihosting library. Output attached.
semihosting_testbench.txt