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

support passing interned string literals #9

Closed
5 tasks
Lotterleben opened this issue Jul 30, 2020 · 0 comments · Fixed by #67
Closed
5 tasks

support passing interned string literals #9

Lotterleben opened this issue Jul 30, 2020 · 0 comments · Fixed by #67
Assignees
Milestone

Comments

@Lotterleben
Copy link
Contributor

Lotterleben commented Jul 30, 2020

currently, binfmt only supports passing string values to a format string, like so:

info!("They said: {:str}", "Hi!"); // on-the-wire = [$index-they, $timestamp, $length, b'H', b'i', b'!']

This is not ideal, as the entire string value ("Hi!") has to be sent over the wire rather than just an index.
Therefore, we'd also like to support passing interned (i.e. predefined) string literals to a format string, like so:

let s: Str = intern!("Hi!");
info!("They said: {:istr}", s); // on-the-wire = [$index-they, $timestamp, $index-hi]

To do this, we'll need to

  • introduce a new formatting parameter (for example {:istr}– preferrably a name that is less similar to the already-existing {:str})
  • extend parser/lib.rs to recognize the new formatting parameter
  • extend parse_args to handle this parameter (implementation note from @japaric: &'t str in them would be pointing into the table: &'t [u8] (like the Format.format string does) not into bytes. Rationale: table lives for as long as the decoder/printer program is running (i.e. effectively it will never be deallocated); on the other hand, bytes may be a temporary stack allocated buffer.)
  • extend docs to explain the difference
  • add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants