seL4Config: remove duplicated constants#547
Conversation
cazb2
force-pushed
the
callumb/improve_config
branch
from
a0b418b to
a5bf957
Compare
|
style check warning can be ignored or we fix the python/style but I was told not too |
| obj_name, size = l.split(": ") | ||
| if "-" in size: | ||
| lhs, rhs = size.strip("( )").split("-", 1) | ||
| size = str(int(lhs.strip(), 0) - int(rhs.strip(), 0)) |
There was a problem hiding this comment.
Hrm, having to do math here is slightly weird. I suppose there's no way around it.
There was a problem hiding this comment.
Unless we extract from part of the compiled tu
| device_regions, | ||
| normal_regions, | ||
| object_sizes, | ||
| object_sizes: Some(object_sizes), |
There was a problem hiding this comment.
Ok, additional cleanup while we're here: can you make this not optional? It's a little silly
There was a problem hiding this comment.
The only benefit of it being an option is testing
There was a problem hiding this comment.
I don't find that very convincing 😅
|
Your branch is out of date, btw. You need to rebase. |
cazb2
force-pushed
the
callumb/improve_config
branch
from
a5bf957 to
eee79fa
Compare
|
|
||
| page_table_index_bits: seL4_PageTableIndexBits | ||
|
|
||
| #ifdef seL4_UserVSpaceTop |
There was a problem hiding this comment.
I believe this #ifdef should be unconditional.
cazb2
force-pushed
the
callumb/improve_config
branch
from
eee79fa to
77ac7e2
Compare
| } | ||
| }; | ||
|
|
||
| let object_sizes = { |
There was a problem hiding this comment.
Is there a reason why you removed the existing
let object_sizes = {
// do stuff to parse
};
pattern here? I can't see what was wrong with it
It's much nicer IMO.
There was a problem hiding this comment.
Hmm interesting I don't see the point in the scope, maybe a function would make more sense?
There was a problem hiding this comment.
The scope makes a new namespace, so you can define several variables without populating global scope.
Also, it logically groups things.
It's sort of like a function in that way yes.
BTW, clipppy failed, and you need to rebase.
There was a problem hiding this comment.
When I look this commit is alr rebased?
There was a problem hiding this comment.
The parent of 711c607 was 'tests: add test cases for bootinfo prefilling', which is several commits behind latest main.
There was a problem hiding this comment.
Yeah, you likely only have the au-ts remote, as tbe parent of that commit is the current au-ts microkit fork. You need to have an additional seL4 remote following seL4/microkit that main tracks: because we work on a fork.
There was a problem hiding this comment.
mmmm so a fork can have a different origin main state?
There was a problem hiding this comment.
I think I understand this a bit now, kinda annoying behaviour
There was a problem hiding this comment.
Yes, they're not linked in any way.
cazb2
force-pushed
the
callumb/improve_config
branch
2 times, most recently
from
dab6f54 to
711c607
Compare
This commit removes some of the duplicated constants definitions that seL4 already exports. Signed-off-by: Callum <c.berry@student.unsw.edu.au>
cazb2
force-pushed
the
callumb/improve_config
branch
from
d12e372 to
f13b9ea
Compare
2 participants
This commit removes some of the duplicated constants definitions that seL4 already exports.