Skip to content

Added a Rust wrapper for parsing cli args using GLib#1286

Merged
stevenengler merged 1 commit intoshadow:devfrom
stevenengler:rust-arg-str-parse
Apr 13, 2021
Merged

Added a Rust wrapper for parsing cli args using GLib#1286
stevenengler merged 1 commit intoshadow:devfrom
stevenengler:rust-arg-str-parse

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler commented Apr 13, 2021

We will call this from Rust when parsing our configuration YAML.

@stevenengler stevenengler added Type: Enhancement New functionality or improved design Component: Main Composing the core Shadow executable labels Apr 13, 2021
@stevenengler stevenengler self-assigned this Apr 13, 2021
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2021

Codecov Report

Merging #1286 (667a2e9) into dev (5a8d430) will increase coverage by 0.02%.
The diff coverage is 92.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1286      +/-   ##
==========================================
+ Coverage   55.54%   55.57%   +0.02%     
==========================================
  Files         141      141              
  Lines       20624    20637      +13     
  Branches     5066     5070       +4     
==========================================
+ Hits        11456    11468      +12     
  Misses       6044     6044              
- Partials     3124     3125       +1     
Flag Coverage Δ
tests 55.57% <92.30%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main/host/process.c 70.80% <92.30%> (+0.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a8d430...667a2e9. Read the comment docs.

@stevenengler stevenengler force-pushed the rust-arg-str-parse branch 7 times, most recently from 729da7e to e6b3203 Compare April 13, 2021 14:04
@stevenengler stevenengler requested a review from robgjansen April 13, 2021 14:12
pub fn process_parseArgStr(
commandLine: *const ::std::os::raw::c_char,
argc: *mut ::std::os::raw::c_int,
argv: *mut *mut *mut ::std::os::raw::c_char,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh didn't know *mut *mut *mut would be a thing.


bool rv = !!g_shell_parse_argv(commandLine, argc, argv, &gError);
if (!rv && gError != NULL && gError->message != NULL) {
*error = strdup(gError->message);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we guaranteed that error is non-null here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I added a check for that.

InterposeMethod process_getInterposeMethod(Process* proc);

// A wrapper around GLib's `g_shell_parse_argv()` that doesn't use GLib types.
bool process_parseArgStr(const char* commandLine, int* argc, char*** argv, char** error);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment on the expectations on the caller for freeing the returned pointers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@stevenengler stevenengler merged commit a9a61b0 into shadow:dev Apr 13, 2021
@stevenengler stevenengler deleted the rust-arg-str-parse branch April 13, 2021 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Main Composing the core Shadow executable Type: Enhancement New functionality or improved design

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants