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

Rename env.rs to args.rs or cli.rs. #1291

Open
jounathaen opened this issue Jun 24, 2024 · 4 comments · May be fixed by #1292
Open

Rename env.rs to args.rs or cli.rs. #1291

jounathaen opened this issue Jun 24, 2024 · 4 comments · May be fixed by #1292

Comments

@jounathaen
Copy link
Member

My association of the name env would be environment variables. However, env.rs is only about kernel parameters/the CLI.

I suggest renaming env.rs to args.rs or cli.rs

@mkroening
Copy link
Member

mkroening commented Jun 24, 2024

This currently follows std::env.

The module is about inspection of the kernel's environment, be it via kernel arguments or environment variables or boot information or similar.

I'd suggest keeping it as is and making sure it's properly documented.

@jounathaen
Copy link
Member Author

But std::env is about Environment variables ("Userspace"). Here we are talking about the kernel "runtime" parameters.

This is in general rather confusing, an example:

  • The struct cli contains a variable env_vars which is filled with four possible environment variables from the kernel command line (in arch::init)
    • -> These are only the "environment variables" relevant for the kernel (these actually aren't environment variables at all).
    • An application can't read them
  • The "real"/application relevant environment variables are retrieved via systemcall (
    let (argc, argv, environ) = syscalls::get_application_parameters();
    ) and then passed to main.

@mkroening
Copy link
Member

But std::env is about Environment variables ("Userspace"). Here we are talking about the kernel "runtime" parameters.

Exactly, std::env is about the environment of the application and hermit::env is about the environment of the kernel, which is a superset of the environment of the application.

Cli parses kernel arguments and provides them as kernel environment variables to other components of the kernel (hermit::env::vars correspond to std::env::vars). The fact that these are provided via kernel arguments instead of any other mechanism is an implementation detail.

The struct cli contains a variable env_vars which is filled with four possible environment variables from the kernel command line (in arch::init)

* -> These are only the "environment variables" relevant for the kernel (these actually aren't environment variables at all).

* An application can't read them

Being a top-level module in does not imply exposition to the user.

  • The "real"/application relevant environment variables are retrieved via systemcall

Yes, most system calls live in hermit::syscalls.

@jounathaen
Copy link
Member Author

I understand the reasoning behind the name, but I still don't think it is a good one. The name is IMHO already strongly associated with another concept. I propose one of the following:

  • runtime_params.rs => e.g., let mhz = runtime_params::freq().ok_or(err: ())?; -> my favorite
  • args.rs => e.g., let mhz = args::freq().ok_or(err: ())?;
  • cli.rs => e.g., let mhz = cli::freq().ok_or(err: ())?; -> Not a big fan of this one myself

@jounathaen jounathaen linked a pull request Jun 24, 2024 that will close this issue
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.

2 participants