Keyboard circular buffer #26
Conversation
Things to consider going forward: - naming conventions between userland and kernel syscall functions. - type-safety problems with the syscall array; this might bite us down the road if registers gets unknowingly trashed. - random userland registers leak into syscall implementations since they aren't nulled out. (See the linux link) - this skeleton is probably terribly slow
| // The way linux does it is to use `struct pt_regs`. | ||
| // https://lwn.net/Articles/750536/ | ||
| // | ||
| // NOTE: arbitrary func ptr for minimum type safety. |
There was a problem hiding this comment.
literally. there is probably some nifty way to do this, but my C-type-signature-fu is lacking.
There was a problem hiding this comment.
har alla syscalls samma function signature? annars är det (void *) som gäller
There was a problem hiding this comment.
nope, de har olika. så (void *) it is
| void debug_interrupt() { | ||
| // NOTE: This will actually throw a general protection fault, since it's a | ||
| // privileged instruction. So we just hook this empty void stub instead and | ||
| // pretend that it works! |
There was a problem hiding this comment.
Gotta save those 0xcc bytes.
Vad händer om compiler:n inline'ar den här tomma funktionen och vi break'ar på den? 😊
There was a problem hiding this comment.
Bra fråga. Min gissning är att symbolen läggs där funktionen inline:as, men är inte säker!
Säkraste är säkert att slänga på en __attribute__ ((noinline)). Jobbigt btw att det är compilerspecifika syntaxer för olika attributes T.T Hade ju varit soft om det bara var noinline void debug_interrupt typ
There was a problem hiding this comment.
Bra fråga. Min gissning är att symbolen läggs där funktionen inline:as, men är inte säker!
Vad händer om den inline'as på mer än ett ställe? :P
Säkraste är säkert att slänga på en attribute ((noinline)). Jobbigt btw att det är compilerspecifika syntaxer för olika attributes T.T Hade ju varit soft om det bara var noinline void debug_interrupt typ
Håller med! Låt oss fixa en ny standard som unifyar usage, ... , 17 competing standards xD
There was a problem hiding this comment.
Vad händer om den inline'as på mer än ett ställe? :P
Symboler är unika (tror jag?) så den kan nog inte vara på flera ställen. Så undefined behavior antagligen :D
Håller med! Låt oss fixa en ny standard som unifyar usage, ... , 17 competing standards xD
Det är ju tempting! IBM jobbar på det: https://www.ibm.com/docs/en/i/7.5?topic=attributes-noinline-function-attribute
| /* asm volatile("int3"); */ | ||
| } | ||
|
|
||
| #define EVER \ |
There was a problem hiding this comment.
HAHAHA! den fick leva kvar. for EVER
There was a problem hiding this comment.
clang-format verkar gilla göra den utdragen också :p
src/userland/userland.c
Outdated
|
|
||
| void strip(uint8_t* s) { | ||
| bool stop = true; | ||
| uint32_t i = strlen(s) - 1; |
There was a problem hiding this comment.
om s = "" så får vi en integer underflow. men då i är unsigned så får vi UINT32_MAX. således är i större än 0 i loopen och random stuff happens.
src/userland/userland.c
Outdated
| bool stop = true; | ||
| uint32_t i = strlen(s) - 1; | ||
| for (; i > 0; i--) { | ||
| if (i == ' ') { |
There was a problem hiding this comment.
-if (i == ' ')
+if (s[i] == ' ')och likaledes för de andra if-blocken
There was a problem hiding this comment.
godnatt yxskaft på den här 🙃
src/userland/userland.c
Outdated
| } | ||
|
|
||
| void memset(uint8_t* p, uint8_t c, uint32_t len) { | ||
| for (uint32_t i = 0; i < len; i++) { |
There was a problem hiding this comment.
invoke med memset(p, c, UINT32_MAX) kommer ge en inf_loop.
generellt sätt är det rekommenderat att inte använda unsigned för loop index, då wrap-around buggar annars inte träffas.
There was a problem hiding this comment.
Den fotpistolen gick jag på 🦶
src/userland/userland.c
Outdated
| for (EVER) { | ||
| uint8_t printl[128]; | ||
| uint8_t readl[128]; | ||
| memset(printl, 0, sizeof printl); |
There was a problem hiding this comment.
-sizeof printl
+sizeof(printl)mer portable C, och känns snyggare, som ett funktions-anrop snarare än något magiskt i språket
src/userland/userland.c
Outdated
| strip(readl); | ||
|
|
||
| if (strcmp(readl, "pwd")) { | ||
| sprintf(&printl[0], "> %s\n", "/"); |
There was a problem hiding this comment.
Exakt, det är ju oftast inte fel iaf ^^
Snart får vi slänga på ett VFS lager
|
I'll merge since the review hopefully identified most of the improvement points. |
No description provided.