SVCD::subdispatch [NOT READY] #34
Conversation
| @@ -1,9 +1,13 @@ | |||
| //This file is included into native.c | |||
| #include "libstormarray.h" | |||
There was a problem hiding this comment.
This will be included in native.c by #30
men in #000000
|
@leonardt why would char not be an 8bit type? |
|
While @leonardt is right and it never hurts to be extra careful, I think it's probably safe to assume that a char is an 8bit type. There are some implementations that will have a 16- or 32-bit char type, but they're usually called differently (e.g. char16_t or char32_t) |
| lua_pushnumber(L, ivkid); | ||
| lua_settable(L, -3); | ||
|
|
||
| }else if (cmd == 0){ //unsubscribe command |
There was a problem hiding this comment.
in svcd.lua, 2 is the unsubscribe command, not 0.
Fantastic Four
There was a problem hiding this comment.
yes, in subdispatch it is. But in SVCD.unsubscribe it is 0 so we changed it here so everything would work
There was a problem hiding this comment.
Was this a bug in unsubscribe?
There was a problem hiding this comment.
I think it was a bug in the program as a whole, I'm not sure which they meant it to be
There was a problem hiding this comment.
I meant it to be 2, but I will fix it when I merge, as this is my bad.
|
Looks good to me (Nick) -Axis |
|
@mschuldt A char is only guaranteed to be at least 8 bits (could be longer). In general it's considered good practice to not assume this (defensive programming, especially in the embedded and systems world, prevents bugs and security concerns). It's the same reason why you use |
|
Your comments were very helpful when going through the code. I like how you kept track of the stack throughout. |
|
Looks good to me. -birdpeople |
|
@jwqchen |
| lua_settable(L, -3); | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Looks good. Good job on doing the comments. Line 224-228 is fantastic! Took me a while to understand the reasoning.
Untitled
| //local cmd = parr:get(1); | ||
| char cmd = parr[0]; | ||
| //local svc_id = parr:get_as(storm.array.UINT16,1); | ||
| int16_t svc_id = *(int16_t*)(parr+1); |
There was a problem hiding this comment.
You cannot do an unaligned access like this on ARM. See my other ranty comments for people doing unaligned writes.
|
This looks nearly ready. Change your uint16_t accesses to be either aligned or bytewise and I will mark it ready |
|
fixes have been pushed |
SVCD.subdispatch