Skip to content
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

Deprecate ROM filename #1787

Closed
ehmry opened this issue Nov 23, 2015 · 18 comments
Closed

Deprecate ROM filename #1787

ehmry opened this issue Nov 23, 2015 · 18 comments
Labels

Comments

@ehmry
Copy link
Contributor

ehmry commented Nov 23, 2015

As mentioned in #1766, the filename argument can be removed and the rightmost session label element treated as the name of the ROM. By the same logic the root argument on File_system sessions could go the same way, such as init -> child -> /.

ehmry added a commit to ehmry/genode that referenced this issue Nov 23, 2015
When names that contain ' -> ' are encountered at Rom_connection the
trailing element is used as the 'filename' argument.

Issue genodelabs#1787
ehmry added a commit to ehmry/genode that referenced this issue Nov 23, 2015
Creates session label strings with the form (label, parent_label).

Issue genodelabs#1787
@nfeske
Copy link
Member

nfeske commented Nov 23, 2015

I am not sure if the same reasoning applies to the root argument of file-system sessions. The root argument allows the client to use knowledge about the file system hierarchy for limiting its own view of the file system. As far as I can see, in contrast to the label, the root argument is not meant as a criterion for taking session-routing decisions or for selecting a policy at the file-system server.

@ehmry
Copy link
Contributor Author

ehmry commented Jan 12, 2016

I've started running into problems with ROM session request buffer overruns from large names being duplicated between the label and filename arguments, so I went ahead and removed the filename argument from ROM session. I wouldn't say that this is a priority issue for me, but I think it does make things a bit simplier.

The following replaces the previous two commits.

ehmry added a commit to ehmry/genode that referenced this issue Jan 12, 2016
New String class for session labels.
Convience function for extracting Label from Arg_string.

Issue genodelabs#1787
ehmry added a commit to ehmry/genode that referenced this issue Jan 12, 2016
Conveying the ROM filename as the final label element simplifies
routing policy and session construction.

Fixes genodelabs#1787
ehmry added a commit to ehmry/genode that referenced this issue Jan 12, 2016
ehmry added a commit to ehmry/genode that referenced this issue Jan 25, 2016
ehmry added a commit to ehmry/genode that referenced this issue Mar 17, 2016
Fixed unsigned integer rollover.

Issue genodelabs#1787
ehmry added a commit to ehmry/genode that referenced this issue Apr 22, 2016
rename "tail" to "last_element" in utilities

Issue genodelabs#1787
@ehmry
Copy link
Contributor Author

ehmry commented May 12, 2016

rebase

ehmry added a commit to ehmry/genode that referenced this issue May 12, 2016
New String class for session labels.
Convience function for extracting Label from Arg_string.

Issue genodelabs#1787
ehmry added a commit to ehmry/genode that referenced this issue May 12, 2016
Conveying the ROM filename as the final label element simplifies
routing policy and session construction.

Fixes genodelabs#1787
@chelmuth
Copy link
Member

You're right, we should consider to integrate the cleanup in the next release.

ehmry added a commit to ehmry/genode that referenced this issue May 23, 2016
Fix the linux dataspace filename retrieval.
Fix the bomb test.

Issue genodelabs#1787
@chelmuth
Copy link
Member

chelmuth commented Jul 1, 2016

I'd like to merge this cleanup. @ehmry could you please squash and rebase these changes on current master?

@ehmry
Copy link
Contributor Author

ehmry commented Jul 5, 2016

Rebased against master. fba1454 is dangerous because it changes the result of ''Session_label(args)'', but I think it is worth it to decouple Session_label from Session_args.

I'm adjusting my Nix work to run against master, and I'll need this to clear to get it to work, otherwise I have to worry about rewriting both label and filename on ROM session requests.

ehmry added a commit to ehmry/genode that referenced this issue Jul 5, 2016
Session_label constructor now takes a bare string rather than a
serialized argument buffer.
Replace all instances of previous constructor with 'label_from_args'
function.

Issue genodelabs#1787
ehmry added a commit to ehmry/genode that referenced this issue Jul 5, 2016
Conveying the ROM filename as the final label element simplifies
routing policy and session construction.

Fixes genodelabs#1787
@ehmry
Copy link
Contributor Author

ehmry commented Jul 5, 2016

@nfeske I've split the compound label constructor to a prefixed_label function as you suggested.

ehmry added a commit to ehmry/genode that referenced this issue Jul 5, 2016
New function 'prefixed_label' for creating compound labels.

Issue genodelabs#1787

fixup fixup
ehmry added a commit to ehmry/genode that referenced this issue Jul 5, 2016
Fix namespacing at rom_prefetcher.

Issue genodelabs#1787
nfeske pushed a commit that referenced this issue Jul 6, 2016
Session_label constructor now takes a bare string rather than a
serialized argument buffer.
Replace all instances of previous constructor with 'label_from_args'
function.

Issue #1787
nfeske pushed a commit that referenced this issue Jul 6, 2016
New function 'prefixed_label' for creating compound labels.

Issue #1787
nfeske added a commit that referenced this issue Jul 6, 2016
Prevent possible integer underflow in 'Session_label::last_element'.

Issue #1787
nfeske added a commit that referenced this issue Jul 6, 2016
Fix potential out-of-bounds access in prefixed_label

Issue #1787
nfeske added a commit that referenced this issue Jul 6, 2016
…ameters)

This change belongs to a different commit.

Issue #1787
nfeske pushed a commit that referenced this issue Jul 6, 2016
Fix namespacing at rom_prefetcher.

Issue #1787
@nfeske
Copy link
Member

nfeske commented Jul 6, 2016

I also added the second part of the change to staging, with the only reservation being the part I annotated inline. @ehmry I would very much appreciate a fixup regarding this part. I nevertheless put it to staging to see what the buildbot/autopilot is saying.

ehmry added a commit to ehmry/genode that referenced this issue Jul 6, 2016
@ehmry
Copy link
Contributor Author

ehmry commented Jul 6, 2016

I've cleanup the ROM label rewriting at fca6e11

ehmry added a commit to ehmry/genode that referenced this issue Jul 6, 2016
@ehmry
Copy link
Contributor Author

ehmry commented Jul 6, 2016

fca6e11 needs fixup 8b8d7a1 :(

nfeske pushed a commit that referenced this issue Jul 6, 2016
nfeske pushed a commit that referenced this issue Jul 6, 2016
@nfeske
Copy link
Member

nfeske commented Jul 6, 2016

Thank you for reworking that part. I added both fixups to staging.

@nfeske
Copy link
Member

nfeske commented Jul 7, 2016

I added a couple of minor fixups to the staging branch. As I was still not fully satisfied with the Child_policy_redirect_rom_file, I tried to simplify it by adding a Session_label::prefix method (commit 78dae09). @ehmry Do you approve?

@ehmry
Copy link
Contributor Author

ehmry commented Jul 7, 2016

Yes, I can think of other situations where ::prefix can be useful.

@nfeske
Copy link
Member

nfeske commented Jul 8, 2016

The never-ending story goes on. ;-) I added a few more fixups to staging. Apart from the usual fixes of the automated tests, I changed the new API functions to avoid the use of pointers. This way, we can successively replace the occurrences of char const * by Session_label const &, e.g., in the connection constructors. It also allows us to use == for comparing strings instead of calling the ugly strcmp function. In the end, we hopefully end up in a situation where the String::string() method is only very rarely needed. The specific commit is 3ff5019

nfeske pushed a commit that referenced this issue Jul 11, 2016
Session_label constructor now takes a bare string rather than a
serialized argument buffer.
Replace all instances of previous constructor with 'label_from_args'
function.

Issue #1787
@nfeske nfeske closed this as completed in 2b8c1af Jul 12, 2016
@nfeske
Copy link
Member

nfeske commented Jul 12, 2016

@ehmry Now that this change entered master, it might be a good time to adapt the components of genode-world to it.

ehmry added a commit to ehmry/genode that referenced this issue Jul 21, 2016
Session_label prefix(a, b) shall not return ' -> b' or 'a -> '.

Issues genodelabs#1787
@ehmry
Copy link
Contributor Author

ehmry commented Jul 21, 2016

I have a small nitpick for the prefix function, d499b29

ehmry added a commit to ehmry/genode that referenced this issue Jul 21, 2016
Use ((!x) || y == "") rather than (!(x && y != "")).

Issue genodelabs#1787
@ehmry
Copy link
Contributor Author

ehmry commented Jul 21, 2016

Swapped the booleans around.

chelmuth pushed a commit that referenced this issue Jul 22, 2016
Session_label prefix(a, b) shall not return ' -> b' or 'a -> '.

Issue #1787
chelmuth pushed a commit that referenced this issue Aug 10, 2016
Session_label prefix(a, b) shall not return ' -> b' or 'a -> '.

Issue #1787
ehmry added a commit to ehmry/genode that referenced this issue Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants