Use designated initializers for 'key_format_tokens' and 'key_tokens' to
allow struct fields to be reordered more easily and to improve
readability.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 security/keys/encrypted-keys/encrypted.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index aef438d18da8..76a6dab2f4d2 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -62,17 +62,17 @@ enum {
 };
 
 static const match_table_t key_format_tokens = {
-	{Opt_default, "default"},
-	{Opt_ecryptfs, "ecryptfs"},
-	{Opt_enc32, "enc32"},
-	{Opt_error, NULL}
+	{ .token = Opt_default, .pattern = "default"},
+	{ .token = Opt_ecryptfs, .pattern = "ecryptfs"},
+	{ .token = Opt_enc32, .pattern = "enc32"},
+	{ .token = Opt_error, .pattern = NULL}
 };
 
 static const match_table_t key_tokens = {
-	{Opt_new, "new"},
-	{Opt_load, "load"},
-	{Opt_update, "update"},
-	{Opt_err, NULL}
+	{ .token = Opt_new, .pattern = "new"},
+	{ .token = Opt_load, .pattern = "load"},
+	{ .token = Opt_update, .pattern = "update"},
+	{ .token = Opt_err, .pattern = NULL}
 };
 
 static bool user_decrypted_data = IS_ENABLED(CONFIG_USER_DECRYPTED_DATA);
-- 
2.51.0

On Thu, Oct 09, 2025 at 01:58:17PM +0200, Thorsten Blum wrote:
> Use designated initializers for 'key_format_tokens' and 'key_tokens' to
> allow struct fields to be reordered more easily and to improve
> readability.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  security/keys/encrypted-keys/encrypted.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index aef438d18da8..76a6dab2f4d2 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -62,17 +62,17 @@ enum {
>  };
>  
>  static const match_table_t key_format_tokens = {
> -	{Opt_default, "default"},
> -	{Opt_ecryptfs, "ecryptfs"},
> -	{Opt_enc32, "enc32"},
> -	{Opt_error, NULL}
> +	{ .token = Opt_default, .pattern = "default"},
> +	{ .token = Opt_ecryptfs, .pattern = "ecryptfs"},
> +	{ .token = Opt_enc32, .pattern = "enc32"},
> +	{ .token = Opt_error, .pattern = NULL}
>  };
>  
>  static const match_table_t key_tokens = {
> -	{Opt_new, "new"},
> -	{Opt_load, "load"},
> -	{Opt_update, "update"},
> -	{Opt_err, NULL}
> +	{ .token = Opt_new, .pattern = "new"},
> +	{ .token = Opt_load, .pattern = "load"},
> +	{ .token = Opt_update, .pattern = "update"},
> +	{ .token = Opt_err, .pattern = NULL}
>  };
>  
>  static bool user_decrypted_data = IS_ENABLED(CONFIG_USER_DECRYPTED_DATA);
> -- 
> 2.51.0
> 

For me this look like a "convert tuple alike initializations into struct
alike initializations" type of change :-)

In a context the change would make sense. E.g., if an optional field was
required.

BR, Jarkko

On Fri, Oct 10, 2025 at 06:55:26AM +0300, Jarkko Sakkinen wrote:
> On Thu, Oct 09, 2025 at 01:58:17PM +0200, Thorsten Blum wrote:
> > Use designated initializers for 'key_format_tokens' and 'key_tokens' to
> > allow struct fields to be reordered more easily and to improve
> > readability.
> > 
> > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > ---
> >  security/keys/encrypted-keys/encrypted.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> > index aef438d18da8..76a6dab2f4d2 100644
> > --- a/security/keys/encrypted-keys/encrypted.c
> > +++ b/security/keys/encrypted-keys/encrypted.c
> > @@ -62,17 +62,17 @@ enum {
> >  };
> >  
> >  static const match_table_t key_format_tokens = {
> > -	{Opt_default, "default"},
> > -	{Opt_ecryptfs, "ecryptfs"},
> > -	{Opt_enc32, "enc32"},
> > -	{Opt_error, NULL}
> > +	{ .token = Opt_default, .pattern = "default"},
> > +	{ .token = Opt_ecryptfs, .pattern = "ecryptfs"},
> > +	{ .token = Opt_enc32, .pattern = "enc32"},
> > +	{ .token = Opt_error, .pattern = NULL}
> >  };
> >  
> >  static const match_table_t key_tokens = {
> > -	{Opt_new, "new"},
> > -	{Opt_load, "load"},
> > -	{Opt_update, "update"},
> > -	{Opt_err, NULL}
> > +	{ .token = Opt_new, .pattern = "new"},
> > +	{ .token = Opt_load, .pattern = "load"},
> > +	{ .token = Opt_update, .pattern = "update"},
> > +	{ .token = Opt_err, .pattern = NULL}
> >  };
> >  
> >  static bool user_decrypted_data = IS_ENABLED(CONFIG_USER_DECRYPTED_DATA);
> > -- 
> > 2.51.0
> > 
> 
> For me this look like a "convert tuple alike initializations into struct
> alike initializations" type of change :-)
> 
> In a context the change would make sense. E.g., if an optional field was
> required.

If we had struct initializers I would equally nak "convert struct
initializers to tuple initializers" type of change.

BR, Jarkko

On Thu, 2025-10-09 at 13:58 +0200, Thorsten Blum wrote:
> Use designated initializers for 'key_format_tokens' and 'key_tokens'
> to allow struct fields to be reordered more easily

How does it improve that?  The key,value pairs are surrounded by braces
so we just cut and paste the lot anyway.

>  and to improve readability.

I don't think I agree with this when looking through the code,
especially because this is the way it's done for *every* option in the
entire key subsystem.  So firstly I really don't think it's helpful for
only encrypted keys to be different from everything else and secondly
when I read the code (as I often do to figure out what the options
mean), the additional .token and .pattern just get in the way of what
I'm looking for.

Regards,

James

On 9. Oct 2025, at 14:44, James Bottomley wrote:
> On Thu, 2025-10-09 at 13:58 +0200, Thorsten Blum wrote:
>> Use designated initializers for 'key_format_tokens' and 'key_tokens'
>> to allow struct fields to be reordered more easily
> 
> How does it improve that?  The key,value pairs are surrounded by braces
> so we just cut and paste the lot anyway.

Using designated initializers (especially for global structs) allows the
fields of struct match_token from linux/parser.h to be reordered or
extended more easily, improving overall maintainability.

>> and to improve readability.
> 
> I don't think I agree with this when looking through the code,
> especially because this is the way it's done for *every* option in the
> entire key subsystem.  So firstly I really don't think it's helpful for
> only encrypted keys to be different from everything else and secondly
> when I read the code (as I often do to figure out what the options
> mean), the additional .token and .pattern just get in the way of what
> I'm looking for.

I just stumbled upon this and didn't check any other files.

I do think it helps with readability, especially for people unfamiliar
with the code. With designated initializers, it's clear what each value
represents without having to look up the definition of 'match_token' in
linux/parser.h.

Thanks,
Thorsten

On Thu, 2025-10-09 at 15:30 +0200, Thorsten Blum wrote:
> On 9. Oct 2025, at 14:44, James Bottomley wrote:
> > On Thu, 2025-10-09 at 13:58 +0200, Thorsten Blum wrote:
> > > Use designated initializers for 'key_format_tokens' and
> > > 'key_tokens' to allow struct fields to be reordered more easily
> > 
> > How does it improve that?  The key,value pairs are surrounded by
> > braces so we just cut and paste the lot anyway.
> 
> Using designated initializers (especially for global structs) allows
> the fields of struct match_token from linux/parser.h to be reordered
> or extended more easily, improving overall maintainability.

Why would we ever want to reorder them?  The reason the ordering is
{token, parser} string is because that's the nicest order to read them
in.

> 
> > > and to improve readability.
> > 
> > I don't think I agree with this when looking through the code,
> > especially because this is the way it's done for *every* option in
> > the entire key subsystem.  So firstly I really don't think it's
> > helpful for only encrypted keys to be different from everything
> > else and secondly when I read the code (as I often do to figure out
> > what the options mean), the additional .token and .pattern just get
> > in the way of what I'm looking for.
> 
> I just stumbled upon this and didn't check any other files.

jejb@lingrow:~/git/linux> git grep 'match_table_t'|wc -l
49

I'll leave it as an exercise to you to figure out how many use the
style you're proposing.

There's definite advantage in uniformity and even if I accepted the
readability argument, which I don't, it's too small a reason to churn
nearly 50 files one at a time.

Regards,

James

On Thu, 2025-10-09 at 09:51 -0400, James Bottomley wrote:
> On Thu, 2025-10-09 at 15:30 +0200, Thorsten Blum wrote:
> > On 9. Oct 2025, at 14:44, James Bottomley wrote:
> > > On Thu, 2025-10-09 at 13:58 +0200, Thorsten Blum wrote:
> > > > Use designated initializers for 'key_format_tokens' and
> > > > 'key_tokens' to allow struct fields to be reordered more easily
> > > 
> > > How does it improve that?  The key,value pairs are surrounded by
> > > braces so we just cut and paste the lot anyway.
> > 
> > Using designated initializers (especially for global structs) allows
> > the fields of struct match_token from linux/parser.h to be reordered
> > or extended more easily, improving overall maintainability.
> 
> Why would we ever want to reorder them?  The reason the ordering is
> {token, parser} string is because that's the nicest order to read them
> in.

I also join James regarding this. I find it fine as it is.

Consider also that there might be patches depending on this change that
cannot be automatically ported to stable kernels. Then, extra work is
required to find which dependencies are needed and backport them as
well.

Thanks

Roberto

> > > > and to improve readability.
> > > 
> > > I don't think I agree with this when looking through the code,
> > > especially because this is the way it's done for *every* option in
> > > the entire key subsystem.  So firstly I really don't think it's
> > > helpful for only encrypted keys to be different from everything
> > > else and secondly when I read the code (as I often do to figure out
> > > what the options mean), the additional .token and .pattern just get
> > > in the way of what I'm looking for.
> > 
> > I just stumbled upon this and didn't check any other files.
> 
> jejb@lingrow:~/git/linux> git grep 'match_table_t'|wc -l
> 49
> 
> I'll leave it as an exercise to you to figure out how many use the
> style you're proposing.
> 
> There's definite advantage in uniformity and even if I accepted the
> readability argument, which I don't, it's too small a reason to churn
> nearly 50 files one at a time.
> 
> Regards,
> 
> James
>