Commit graph

29 commits

Author SHA1 Message Date
Alexandre Courbot
824a7938b2 serde_keyvalue: add tests for outer-braces in arguments
Although our arguments are a struct, specifying braces for it should not
be allowed. Add tests to enforce that.

BUG=None
TEST=cargo test -p serde_keyvalue

Change-Id: Ic4e731999269895e0c4a65e66cd338500c520451
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4600499
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
2023-06-09 04:59:54 +00:00
Alexandre Courbot
68d895fdf6 serde_keyvalue: fix parsing of flattened structs
Outer structs that contain a flattened element are not parsed with
`deserialize_struct`, which meant the `top_struct_parsed` remained
false. Fix that by also flipping it in `deserialize_map`, which is used
for the flattened top-level structs.

BUG=b:217480278
TEST=cargo test -p serde_keyvalue

Change-Id: I2288361d63de21de7f55ee5ffbd130ee562d2df2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4596441
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2023-06-09 02:59:31 +00:00
Alexandre Courbot
f8458abf49 serde_keyvalue: allow structs with default values to be left unspecified
Allow optional structs which members all have default values to have
their values omitted. This is convenient when we want to specify a flag
that can take optional extra arguments.

BUG=b:217480278
TEST=cargo test -p serde_keyvalue

Change-Id: If2920834f3221146439fc6b8ac54cd9cb215cb9e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4596440
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2023-06-09 02:58:41 +00:00
Alexandre Courbot
82f3e5a4e4 serde_keyvalue: fix clippy warnings
BUG=None
TEST=cargo build

Change-Id: I83d28839ccfdebcd7a3b2ec3db0b5948fde5813c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4583702
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2023-06-05 05:58:38 +00:00
Daniel Verkamp
ac0fc378a3 Fix remaining Chrome/Chromium OS instances
These should be written as ChromeOS and ChromiumOS (without the space)
to match the updated branding. The copyright headers were already
migrated to the new style (https://crrev.com/c/3894243), but there were
some more instances left over.

BUG=None
TEST=tools/cargo-doc

Change-Id: I8c76aea2eb33b2e370ab71ee9b5cc0a4cfd00585
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4129934
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2023-01-03 22:14:30 +00:00
Daniel Verkamp
10990c89af Rust 1.65: Fix clippy derive_partial_eq_without_eq lints
BUG=b:260784028
TEST=tools/clippy

Change-Id: Ib2b595385ed04b9480b22549334ce798d980d347
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4064717
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
2022-12-01 01:32:30 +00:00
Alexandre Courbot
03cf81d1b5 serde_keyvalues: perform integer detection up to separator character
Integer detection was stopping and succeeding at the first non-integer
character, i.e. "0-4" would be detected as integer "0" and leave "-4" as
the remainder string, meaning the parser would fail later.

Improve this by requesting integer detection to also detect a separator
character and fail if the whole input until that separator is not a
valid number. That way, "0-4" fails to parse as a number, and
deserialize_any will instead properly recognize it as a string.

BUG=b:255223604
TEST=cargo test -p serde_keyvalues

Change-Id: I6b5805c7b0b82c36f61280c26b9f92f219196511
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4060599
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-12-01 01:26:13 +00:00
Daniel Verkamp
a62ad8df75 Rust 1.65: Fix clippy get_first lints
BUG=b:260784028
TEST=tools/clippy

Change-Id: If718d168aa84f5fdb80accf3e9993ce151a7da33
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4064715
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2022-11-30 19:39:40 +00:00
Alexandre Courbot
171752d9ee serde_keyvalue: fix parsing of quoted strings in a sequence
Parsing a quoted string as the last element of a sequence would fail
since ']' was not recognized as a separator. Fix this and factorize the
code recognizing separators into a single function to prevent this kind
of issue from happening again in the future.

BUG=None
TEST=cargo test -p serde_keyvalue

Change-Id: I1367da843787c40fb6c15a615d7cd036bddd1381
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4054812
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-11-29 00:24:44 +00:00
Alexandre Courbot
e288105e56 serde_keyvalue: allow parsing of inner structs
Structs can be parsed as part of the command-line input, if they are
enclosed within braces.

BUG=b:218223240
TEST=cargo test -p serde_keyvalue

Change-Id: I05d9d1237036c6ba464408d56c216072e285d801
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3979490
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Pujun Lun <lunpujun@google.com>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
2022-10-26 17:41:37 +00:00
Alexandre Courbot
eb8d6c1462 serde_keyvalue: clarify map key parsing
Group related bits closer and comment a bit. This does not change the
behavior of the code.

BUG=None
TEST=cargo test -p serde_keyvalue

Change-Id: I01136fbebaa3790311255492f87848e058a8ae8f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3979489
Reviewed-by: Pujun Lun <lunpujun@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
2022-10-26 17:41:37 +00:00
Allen Webb
07041ace37 Adding missing # provided by ebuild to Cargo.tomls
cros_async and by extension serde_keyvalue are needed on ChromeOS, so
add the `# provided by ebuild` comments where appropriate to make it
possible to uprev cros_async (which requires pulling in serde_keyvalue).

BUG=b:250883877
TEST=kokoro passes

Change-Id: I423cac0c4e5e30afac058b914358caca7d27edbf
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3953082
Auto-Submit: Allen Webb <allenwebb@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-10-14 16:42:38 +00:00
Alexandre Courbot
e91b0fd604 serde_keyvalue: support for tuple and struct enums
Up to now only unit enums were supported. Add support for tuple and
struct enums, which can deserialize the following enum

    enum VideoMode {
        Fullscreen,
        Window(u32, u32),
        Remote { host: String },
    }

from the following inputs:

    mode=fullscreen
    mode=window[640,480]
    mode=remote[host=workstation]

This allows us to stop using flattened or dedicated enums for parsing and also
results in a less ambiguous syntax for these cases.

BUG=b:248993755
TEST=cargo test -p serde_keyvalue
TEST=cargo test

Change-Id: I142fc91fab19f4a977adec3ee36094e5f95363db
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3915040
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-10-05 07:03:48 +00:00
Alexandre Courbot
3573dfef69 serde_keyvalue: add support for sequences and sets
Sequences and sets can be supported by implementing the SeqAccess trait.
This opens up a few new scenarios for the command-line argument parser,
like support for tuples and flags collected into a set.

For instance, the following struct:

    struct Layout {
        resolution: (u16, u16),
        scanlines: Vec<u16>,
    }

Can be built from the following input:

    resolution=[320,200],scanlines=[0,64,128]

BUG=b:248993755
TEST=cargo test -p serde_keyvalue
TEST=cargo test

Change-Id: Ie8972807ed6b171f5b65f6d5b6d458a2cbf450a5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3915039
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-10-05 06:57:37 +00:00
Alexandre Courbot
aa81c96fd1 serde_keyvalue: make unquoted strings stop at the next bracket
In order to support tuples and enum variants, we need to add an extra
separator to specify the bounds of the tuple or enum. `[` and `]` are
good candidates since they are not often met in strings and are not
reserved by the shell, contrary to `(` and `)`.

BUG=b:248993755
TEST=cargo test -p serde_keyvalue
TEST=cargo test

Change-Id: I6743461f0cd3af0bce11e4978fa14776048170a5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3915038
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-10-05 06:55:56 +00:00
Alexandre Courbot
a6396869a4 serde_keyvalue: fix cargo doc warning
TEST=cargo doc

Change-Id: I7ef54dc973aa279222214b5dc08579f2a9973ee0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3918032
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
2022-09-28 02:16:40 +00:00
Alexandre Courbot
e1c1719b84 serde_keyvalue: remove enum wrapper
This wrapper type is useless as we can just implement the access traits
on KeyValueDeserializer directly.

BUG=b:218223240
TEST=cargo test -p serde_keyvalue

Change-Id: I5013b3fea2f0a2febf182a2fd42037d1cbdbe737
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3915037
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
2022-09-26 22:25:28 +00:00
Alexandre Courbot
86210e591e serde_keyvalue: fix error reporting offset
After deserializing a value, do not consume the next character until we
confirm it is the expected delimiter, otherwise the "expected comma"
error will be reported with an extra offset of 1.

TEST=cargo test -p serde_keyvalues

Change-Id: Id3b80a256bfee51eb1125091dcb0114c2e4af226
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3905070
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-09-21 16:50:09 +00:00
Alexandre Courbot
aad77ff1e2 serde_keyvalue: add a few tests
Add tests for a unit type and an untagged enum.

TEST=cargo test -p serde_keyvalue

Change-Id: Ifb840e3270624ef0e54d69b7eba2e631ffcac4e5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3906752
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-09-21 02:57:52 +00:00
Alexandre Courbot
b68d4c4db0 serde_keyvalue: add documentation about caveats of using flatten
Serde's `flatten` property is very useful to transparently add
parameters to a command-line argument, but it comes with a few
important limitations. Document them and discourage its use whenever
possible.

BUG=b:218223240
BUG=b:241300017
TEST=cargo build

Change-Id: I822645acbf0ef563d0c0d14f719571093a5152f2
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3889327
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-09-15 02:07:56 +00:00
Alexandre Courbot
7468fa4f91 serde_keyvalue: export KeyValueDeserializer
We will need to control deserialization from a lower-level in order to
remove the `flatten` attribute from the VhostUserParams struct, so make
the KeyValueDeserializer interface available publicly.

BUG=b:217480043
TEST=cargo build

Change-Id: I366a71ef1b4f38ba4673478dc7ae8b928a8f57d3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3889328
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-09-15 02:07:56 +00:00
Alexandre Courbot
92918a5b01 serde_keyvalue: use nom-based methods for deserialize_any
These methods are better to predict the type of what follows than just
parsing the next character. For instance, '1foo' was interpreted as an
integer whereas it should be a string.

BUG=b:218223240
BUG=b:241300017
TEST=cargo test -p serde_keyvalue
TEST=cargo test

Change-Id: I9a0a3cb73e06783784d47450de7d291adf41aa53
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3889330
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-09-15 02:07:56 +00:00
Dennis Kempin
1dab58a2cf Update all copyright headers to match new style
This search/replace updates all copyright notices to drop the
"All rights reserved", Use "ChromiumOS" instead of "Chromium OS"
and drops the trailing dots.

This fulfills the request from legal and unifies our notices.

./tools/health-check has been updated to only accept this style.

BUG=b:246579983
TEST=./tools/health-check

Change-Id: I87a80701dc651f1baf4820e5cc42469d7c5f5bf7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3894243
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
2022-09-13 18:41:29 +00:00
Alexandre Courbot
8b75feb12f serde_keyvalue: more natural signed number parsing
Support parsing negative numbers with a specified radix as e.g.
`-0xff00` instead of `0x-ff00`.  This is a potentially breaking change
but I don't believe anyone made use of the former syntax anyway.

The new number parser has the nice side-effect of removing some extra
code, and also signaling parsing errors more accurately, so that's an
added bonus. On the other hand we lose the distinction between "invalid
number" and "expected number", but that distinction was dubious anyway
so losing it is for the better IMHO.

BUG=b:218223240
TEST=cargo test -p serde_keyvalue

Change-Id: I6b2943ecd0df92071afe0682b339dcf5b59e2826
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3853315
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Tatsuyuki Ishi <ishitatsuyuki@google.com>
2022-08-27 03:27:48 +00:00
Alexandre Courbot
7e0206ee4a serde_keyvalue: use nom crate for parsing
Use the nom crate for parsing. This will arguably be both simpler to
read and more reliable than our current ad-hoc parser.

Add a few extra tests to enforce the new rules that the new parser is
supposed to enforce.

BUG=b:218223240
TEST=cargo test -p serde_keyvalues -p crosvm
TEST=ARCVM starts successfully.

Change-Id: I4ea188ffe8aee872071c900793c50127855403ec
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3822428
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-08-24 00:37:59 +00:00
Daniel Verkamp
600ad38ff8 Remove redundant {self} imports
- Remove trailing ::{self} on all use statements
- Remove any resulting single-level use statements (e.g. use libc;)
- Reformat with `tools/fmt --nightly`

BUG=b:239937122
TEST=tools/dev_container tools/presubmit --all

Change-Id: I8afd1b0458ca6d08d9b41a24583f7d4148597ccb
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3798973
Auto-Submit: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
2022-08-01 21:27:54 +00:00
Dennis Kempin
4fea399df9 Reformat imports
crosvm is switching the import style to use one import per line.
While more verbose, this will greatly reduce the occurence of merge
conflicts going forward.

Note: This is using a nightly feature of rustfmt. So it's a one-off
re-format only. We are considering adding a nightly toolchain to
enable the feature permanently.

BUG=b:239937122
TEST=CQ

Change-Id: Id2dd4dbdc0adfc4f8f3dd1d09da1daafa2a39992
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3784345
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
2022-07-28 00:15:50 +00:00
Mike Gerow
b53a156fbc serde_keyvalue: parse hex, octal, and binary nums
This adds support for parsing hex `m=0x1234abcd` octal `0o755` and
binary `m=0o1100` values according the their prefix. Negative numbers
can also be specified by adding a `-` after the prefix.

Note that negative values like `m=-0x1234` don't work, largely as an
artifact of the way the Num trait works. This could probably be fixed,
but given hex, octal, and binary numbers tend to be used in unsigned
situations most of the time this shouldn't be an issue.

The main motivation for this is the debugcon serial device, which
accepts an argument which is an x86 IO port. These are much more
naturally described in hex.

BUG=b:233610263

Change-Id: Ic68bacd772de6aebfaad0de7b8aa6faf5d4c1555
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3671595
Commit-Queue: Mike Gerow <gerow@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
2022-06-02 22:03:20 +00:00
Alexandre Courbot
aa043e8c00 add serde_keyvalue crate
Add a crate for deserializing command-line options given as key-values.

This crate leverages serde to deserialize key-value argument strings (a
commonly used pattern in crosvm) into configuration structures. This
will allow us to remove a big part of the manual parsing currently done
in `main.rs`, will provide consistent arguments to the `crosvm run` and
`crosvm device` commands, and results in more precise error reporting.

The use of serde will also allow us to similarly deserialize
configuration structures from configuration files with very little extra
code involved.

As explained in the crate's documentation, its main entry point is a
`from_key_values` function that allows a build a struct implementing
serde's `Deserialize` trait from a key/values string.

In order to integrate transparently with `argh`, there is also a
`FromKeyValue` derive macro that automatically implements `FromArgValue`
for structs deriving from it.

BUG=b:218223240
BUG=b:217480043
TEST=cargo build
TEST=cargo test -p serde_keyvalue

Change-Id: Id6316e40150d5f08a05e6f04e39ecbc73d72dfa0
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3439669
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Anton Romanov <romanton@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-03-24 01:33:21 +00:00