diff mbox series

[v1,07/28] rust: macros: take string literals in `module!`

Message ID 20221110164152.26136-8-ojeda@kernel.org
State New
Headers
Series Rust core additions |

Commit Message

Miguel Ojeda Nov. 10, 2022, 4:41 p.m. UTC 
  From: Gary Guo <gary@garyguo.net>

Instead of taking binary string literals, take string ones instead,
making it easier for users to define a module, i.e. instead of
calling `module!` like:

    module! {
        ...
        name: b"rust_minimal",
        ...
    }

now it is called as:

    module! {
        ...
        name: "rust_minimal",
        ...
    }

Module names, aliases and license strings are restricted to
ASCII only. However, the author and the description allows UTF-8.

For simplicity (avoid parsing), escape sequences and raw string
literals are not yet handled.

Link: https://github.com/Rust-for-Linux/linux/issues/252
Link: https://lore.kernel.org/lkml/YukvvPOOu8uZl7+n@yadro.com/
Signed-off-by: Gary Guo <gary@garyguo.net>
[Reworded, adapted for upstream and applied latest changes]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 rust/macros/helpers.rs       | 24 ++++++++++++++++++------
 rust/macros/lib.rs           | 12 ++++++------
 rust/macros/module.rs        | 10 +++++-----
 samples/rust/rust_minimal.rs |  8 ++++----
 samples/rust/rust_print.rs   |  8 ++++----
 5 files changed, 37 insertions(+), 25 deletions(-)

Comments

Wei Liu Nov. 14, 2022, 2:47 p.m. UTC | #1 
On Thu, Nov 10, 2022 at 05:41:19PM +0100, Miguel Ojeda wrote:
> From: Gary Guo <gary@garyguo.net>
> 
> Instead of taking binary string literals, take string ones instead,
> making it easier for users to define a module, i.e. instead of
> calling `module!` like:
> 
>     module! {
>         ...
>         name: b"rust_minimal",
>         ...
>     }
> 
> now it is called as:
> 
>     module! {
>         ...
>         name: "rust_minimal",
>         ...
>     }
> 
> Module names, aliases and license strings are restricted to
> ASCII only. However, the author and the description allows UTF-8.

What's the rationale behind allowing UTF-8? Why not stick with ASCII
only?

Thanks,
Wei.
Miguel Ojeda Nov. 14, 2022, 4:46 p.m. UTC | #2 
On Mon, Nov 14, 2022 at 3:47 PM Wei Liu <wei.liu@kernel.org> wrote:
>
> What's the rationale behind allowing UTF-8? Why not stick with ASCII
> only?

The reason is that there are already some cases on the C side.

For authors, there are about 158 non-ASCII in the kernel tree (if I
grepped correctly), e.g.:

    MODULE_AUTHOR("漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>");
    MODULE_AUTHOR("Martin Povišer <povik+lin@cutebit.org>");
    MODULE_AUTHOR("Jérôme Pouiller <jerome.pouiller@silabs.com>");
    MODULE_AUTHOR("Uwe Kleine-König <u.kleine-koenig@pengutronix.de>");

There are also a few descriptions too, e.g.:

    MODULE_DESCRIPTION("NAND flash driver for OLPC CAFÉ chip");
    MODULE_DESCRIPTION("NHPoly1305 ε-almost-∆-universal hash function");

Cheers,
Miguel
Wei Liu Nov. 14, 2022, 5:25 p.m. UTC | #3 
On Mon, Nov 14, 2022 at 05:46:05PM +0100, Miguel Ojeda wrote:
> On Mon, Nov 14, 2022 at 3:47 PM Wei Liu <wei.liu@kernel.org> wrote:
> >
> > What's the rationale behind allowing UTF-8? Why not stick with ASCII
> > only?
> 
> The reason is that there are already some cases on the C side.
> 
> For authors, there are about 158 non-ASCII in the kernel tree (if I
> grepped correctly), e.g.:
> 
>     MODULE_AUTHOR("漆鹏振 (Qi Pengzhen) <aric.pzqi@ingenic.com>");
>     MODULE_AUTHOR("Martin Povišer <povik+lin@cutebit.org>");
>     MODULE_AUTHOR("Jérôme Pouiller <jerome.pouiller@silabs.com>");
>     MODULE_AUTHOR("Uwe Kleine-König <u.kleine-koenig@pengutronix.de>");
> 
> There are also a few descriptions too, e.g.:
> 
>     MODULE_DESCRIPTION("NAND flash driver for OLPC CAFÉ chip");
>     MODULE_DESCRIPTION("NHPoly1305 ε-almost-∆-universal hash function");

Okay. That's fair enough.

Thanks,
Wei.

> 
> Cheers,
> Miguel
diff mbox series

Patch

diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index cdc7dc6135d2..cf7ad950dc1e 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -18,10 +18,16 @@  pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
     }
 }
 
-pub(crate) fn try_byte_string(it: &mut token_stream::IntoIter) -> Option<String> {
-    try_literal(it).and_then(|byte_string| {
-        if byte_string.starts_with("b\"") && byte_string.ends_with('\"') {
-            Some(byte_string[2..byte_string.len() - 1].to_string())
+pub(crate) fn try_string(it: &mut token_stream::IntoIter) -> Option<String> {
+    try_literal(it).and_then(|string| {
+        if string.starts_with('\"') && string.ends_with('\"') {
+            let content = &string[1..string.len() - 1];
+            if content.contains('\\') {
+                panic!("Escape sequences in string literals not yet handled");
+            }
+            Some(content.to_string())
+        } else if string.starts_with("r\"") {
+            panic!("Raw string literals are not yet handled");
         } else {
             None
         }
@@ -40,8 +46,14 @@  pub(crate) fn expect_punct(it: &mut token_stream::IntoIter) -> char {
     }
 }
 
-pub(crate) fn expect_byte_string(it: &mut token_stream::IntoIter) -> String {
-    try_byte_string(it).expect("Expected byte string")
+pub(crate) fn expect_string(it: &mut token_stream::IntoIter) -> String {
+    try_string(it).expect("Expected string")
+}
+
+pub(crate) fn expect_string_ascii(it: &mut token_stream::IntoIter) -> String {
+    let string = try_string(it).expect("Expected string");
+    assert!(string.is_ascii(), "Expected ASCII string");
+    string
 }
 
 pub(crate) fn expect_end(it: &mut token_stream::IntoIter) {
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index e40caaf0a656..c1d385e345b9 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -25,20 +25,20 @@  use proc_macro::TokenStream;
 ///
 /// module!{
 ///     type: MyModule,
-///     name: b"my_kernel_module",
-///     author: b"Rust for Linux Contributors",
-///     description: b"My very own kernel module!",
-///     license: b"GPL",
+///     name: "my_kernel_module",
+///     author: "Rust for Linux Contributors",
+///     description: "My very own kernel module!",
+///     license: "GPL",
 ///     params: {
 ///        my_i32: i32 {
 ///            default: 42,
 ///            permissions: 0o000,
-///            description: b"Example of i32",
+///            description: "Example of i32",
 ///        },
 ///        writeable_i32: i32 {
 ///            default: 42,
 ///            permissions: 0o644,
-///            description: b"Example of i32",
+///            description: "Example of i32",
 ///        },
 ///    },
 /// }
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 186a5b8be23c..a7e363c2b044 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -108,11 +108,11 @@  impl ModuleInfo {
 
             match key.as_str() {
                 "type" => info.type_ = expect_ident(it),
-                "name" => info.name = expect_byte_string(it),
-                "author" => info.author = Some(expect_byte_string(it)),
-                "description" => info.description = Some(expect_byte_string(it)),
-                "license" => info.license = expect_byte_string(it),
-                "alias" => info.alias = Some(expect_byte_string(it)),
+                "name" => info.name = expect_string_ascii(it),
+                "author" => info.author = Some(expect_string(it)),
+                "description" => info.description = Some(expect_string(it)),
+                "license" => info.license = expect_string_ascii(it),
+                "alias" => info.alias = Some(expect_string_ascii(it)),
                 _ => panic!(
                     "Unknown key \"{}\". Valid keys are: {:?}.",
                     key, EXPECTED_KEYS
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 54ad17685742..dc05f4bbe27e 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -6,10 +6,10 @@  use kernel::prelude::*;
 
 module! {
     type: RustMinimal,
-    name: b"rust_minimal",
-    author: b"Rust for Linux Contributors",
-    description: b"Rust minimal sample",
-    license: b"GPL",
+    name: "rust_minimal",
+    author: "Rust for Linux Contributors",
+    description: "Rust minimal sample",
+    license: "GPL",
 }
 
 struct RustMinimal {
diff --git a/samples/rust/rust_print.rs b/samples/rust/rust_print.rs
index 09f737790f3f..8b39d9cef6d1 100644
--- a/samples/rust/rust_print.rs
+++ b/samples/rust/rust_print.rs
@@ -7,10 +7,10 @@  use kernel::prelude::*;
 
 module! {
     type: RustPrint,
-    name: b"rust_print",
-    author: b"Rust for Linux Contributors",
-    description: b"Rust printing macros sample",
-    license: b"GPL",
+    name: "rust_print",
+    author: "Rust for Linux Contributors",
+    description: "Rust printing macros sample",
+    license: "GPL",
 }
 
 struct RustPrint;