[PATCH v2 0/2] Allow multiple keyspecs in one environment variable

[PATCH v2 0/2] Allow multiple keyspecs in one environment variable

[Jonas Rebmann]

This contains the actual change to keytoc as well a migration Note.

Allowing any number of public keys to be provided via a single
environment variable eases integration in more complex setups where
multiple public keys per keyring are managed externally.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Changes in v2:
- Split up the keyspec environment variables when multiple are provided
  too.
- Use strsep_unescape instead of manual split/unescape
- Remove RFC tag
- Link to v1: https://lore.barebox.org/barebox/20260206-keytoc-multi-env-v1-1-638fbf2b3634@pengutronix.de

---
Jonas Rebmann (2):
      crypto: keytoc: Split env-provided full keyspec on spaces
      Documentation: migration-guides: Document change in keyspec env vars

 .../migration-guides/migration-master.rst          | 17 ++++++
 scripts/include/linux/string.h                     |  2 +-
 scripts/include/string_util.h                      | 65 ++++++++++++++++++++
 scripts/keytoc.c                                   | 71 +++++++++++++++-------
 4 files changed, 132 insertions(+), 23 deletions(-)
---
base-commit: 74393178faf08f8102c7c370ac310c29112e1137
change-id: 20260206-keytoc-multi-env-4a3300292e4a

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

[PATCH v2 1/2] crypto: keytoc: Split env-provided full keyspec on spaces

[Jonas Rebmann]

keytoc/CONFIG_CRYPTO_PUBLIC_KEYS can work with a complete keyspec
provided by an environment variable as opposed to providing single URIs.
This would be a very useful feature if it could also provide any number
of keys. Kconfig however provides keytoc with regular keyspecs already
split at spaces so without furhter measures, the env variable can only
be expanded into a single key.

If a complete argument is provided via __ENV, split it at any space
character that is not escaped with a backslash in front of it.  An
actual backslash in a path needs to be escape with another backslash.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/include/linux/string.h |  2 +-
 scripts/include/string_util.h  | 65 ++++++++++++++++++++++++++++++++++++++
 scripts/keytoc.c               | 71 +++++++++++++++++++++++++++++-------------
 3 files changed, 115 insertions(+), 23 deletions(-)

diff --git a/scripts/include/linux/string.h b/scripts/include/linux/string.h
index 649287b80a..ac06f96d8d 100644
--- a/scripts/include/linux/string.h
+++ b/scripts/include/linux/string.h
@@ -16,4 +16,4 @@ int strtobool(const char *s, bool *res);
 extern size_t strlcpy(char *dest, const char *src, size_t size);
 #endif
 
-#endif /* _LINUX_STRING_H_ */
+#endif /* _TOOLS_LINUX_STRING_H_ */
diff --git a/scripts/include/string_util.h b/scripts/include/string_util.h
new file mode 100644
index 0000000000..e71aa60d26
--- /dev/null
+++ b/scripts/include/string_util.h
@@ -0,0 +1,65 @@
+#ifndef _TOOLS_STRING_UTIL_H_
+#define _TOOLS_STRING_UTIL_H_
+
+#include <linux/types.h>
+#include <stddef.h>
+
+// SPDX-SnippetBegin
+// SPDX-Snippet-Comment: Origin-URL: https://git.pengutronix.de/cgit/barebox/tree/lib/string.c?id=dfcf686f94a5a5387660f2afab79a714baab828a
+
+/**
+ * strsep_unescaped - Split a string into tokens, while ignoring escaped delimiters
+ * @s: The string to be searched
+ * @ct: The delimiter characters to search for
+ * @delim: optional pointer to store found delimiter into
+ *
+ * strsep_unescaped() behaves like strsep unless it meets an escaped delimiter.
+ * In that case, it shifts the string back in memory to overwrite the escape's
+ * backslash then continues the search until an unescaped delimiter is found.
+ *
+ * On end of string, this function returns NULL. As long as a non-NULL
+ * value is returned and @delim is not NULL, the found delimiter will
+ * be stored into *@delim.
+ */
+static char *strsep_unescaped(char **s, const char *ct, char *delim)
+{
+	char *sbegin = *s, *hay;
+	const char *needle;
+	size_t shift = 0;
+
+	if (sbegin == NULL)
+		return NULL;
+
+	for (hay = sbegin; *hay != '\0'; ++hay) {
+		*hay = hay[shift];
+
+		if (*hay == '\\') {
+			*hay = hay[++shift];
+			if (*hay != '\\')
+				continue;
+		}
+
+		for (needle = ct; *needle != '\0'; ++needle) {
+			if (*hay == *needle)
+				goto match;
+		}
+	}
+
+	*s = NULL;
+	if (delim)
+		*delim = '\0';
+	return sbegin;
+
+match:
+	if (delim)
+		*delim = *hay;
+	*hay = '\0';
+	*s = &hay[shift + 1];
+
+	return sbegin;
+}
+
+// SPDX-SnippetEnd
+
+
+#endif /* _TOOLS_STRING_UTIL_H_ */
diff --git a/scripts/keytoc.c b/scripts/keytoc.c
index 77ada3af45..ed091285f4 100644
--- a/scripts/keytoc.c
+++ b/scripts/keytoc.c
@@ -6,9 +6,12 @@
  * URI to a C struct suitable to compile with barebox.
  *
  * TODO: Find a better way for reimport_key()
- *
  */
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */
+
+#include "include/string_util.h"
+
 #include <stdio.h>
 #include <string.h>
 #include <time.h>
@@ -784,8 +787,13 @@ static bool parse_info(char *p, struct keyinfo *out)
 	}
 }
 
-static bool get_name_path(const char *keyspec, struct keyinfo *out)
+static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
 {
+	if (!strncmp(keyspec, "pkcs11:", 7)) { /* legacy format of pkcs11 URI */
+		out->path = strdup(keyspec);
+		return true;
+	}
+
 	char *sep, *spec;
 
 	spec = strdup(keyspec);
@@ -814,10 +822,10 @@ static bool get_name_path(const char *keyspec, struct keyinfo *out)
 
 int main(int argc, char *argv[])
 {
-	int i, opt, ret;
+	int argi, opt, ret;
 	char *outfile = NULL;
-	int keycount;
-	struct keyinfo *keylist;
+	size_t keycount, num_positionals;
+	struct keyinfo *keylist = NULL;
 
 	outfilep = stdout;
 
@@ -852,22 +860,41 @@ int main(int argc, char *argv[])
 		exit(1);
 	}
 
-	keycount = argc - optind;
-	keylist = calloc(sizeof(struct keyinfo), keycount);
 
-	for (i = 0; i < keycount; i++) {
-		const char *keyspec = try_resolve_env(argv[optind + i]);
-		struct keyinfo *info = &keylist[i];
+	num_positionals = argc - optind;
+	keycount = num_positionals;
 
-		if (!keyspec)
-			exit(1);
+	keylist = calloc(keycount, sizeof(*keylist));
+
+	if (!keylist)
+		enomem_exit("push");
+
+	int listi = 0;
+
+	for (argi = 0; argi < num_positionals; argi++) {
+		char *arg = strdup(argv[optind + argi]);
+		char *resolved = try_resolve_env(arg);
 
-		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
-			info->path = strdup(keyspec);
+		if (arg == resolved) {
+			keylist[listi].path = arg;
+			listi++;
 		} else {
-			if (!get_name_path(keyspec, info)) {
-				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
-				exit(1);
+			char *keyspecs = strdup(resolved);
+			char *keyspec;
+
+			keycount--;
+			while ((keyspec = strsep_unescaped(&keyspecs, " ", NULL))) {
+				keycount++;
+				keylist = reallocarray(keylist, keycount, sizeof(*keylist));
+				if (!keylist)
+					enomem_exit("realloc keylist");
+				bzero(keylist + (keycount - 1), sizeof(*keylist));
+				if (!parse_keyspec(keyspec, &keylist[listi])) {
+					fprintf(stderr, "invalid keyspec %i: %s\n", optind,
+						keyspec);
+					exit(1);
+				}
+				listi++;
 			}
 		}
 	}
@@ -885,14 +912,14 @@ int main(int argc, char *argv[])
 	}
 
 
-	for (i = 0; i < keycount; i++) {
-		struct keyinfo *info = &keylist[i];
+	for (argi = 0; argi < keycount; argi++) {
+		struct keyinfo *info = &keylist[argi];
 
 		/* resolve __ENV__ for name_hint and path */
 		info->name_hint = try_resolve_env(info->name_hint);
 		info->path = try_resolve_env(info->path);
 
-		if (asprintf(&info->name_c, "key_%i", i + 1) < 0)
+		if (asprintf(&info->name_c, "key_%i", argi + 1) < 0)
 			enomem_exit("asprintf");
 
 		/* unfortunately, the fit name hint is mandatory in the barebox codebase */
@@ -901,7 +928,7 @@ int main(int argc, char *argv[])
 
 		if (!info->keyring) {
 			info->keyring = strdup("fit");
-			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + i]);
+			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + argi]);
 		}
 
 		ret = gen_key(info);

-- 
2.51.2.535.g419c72cb8a

Re: [PATCH v2 1/2] crypto: keytoc: Split env-provided full keyspec on spaces

[Marco Felsch]

On 26-02-18, Jonas Rebmann wrote:
> keytoc/CONFIG_CRYPTO_PUBLIC_KEYS can work with a complete keyspec
> provided by an environment variable as opposed to providing single URIs.
> This would be a very useful feature if it could also provide any number
> of keys. Kconfig however provides keytoc with regular keyspecs already
> split at spaces so without furhter measures, the env variable can only
> be expanded into a single key.
> 
> If a complete argument is provided via __ENV, split it at any space
> character that is not escaped with a backslash in front of it.  An
> actual backslash in a path needs to be escape with another backslash.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  scripts/include/linux/string.h |  2 +-
>  scripts/include/string_util.h  | 65 ++++++++++++++++++++++++++++++++++++++
>  scripts/keytoc.c               | 71 +++++++++++++++++++++++++++++-------------

Porting strsep_unescaped should be one patch and the keytoc changes
should be one patch.

>  3 files changed, 115 insertions(+), 23 deletions(-)
> 
> diff --git a/scripts/include/linux/string.h b/scripts/include/linux/string.h
> index 649287b80a..ac06f96d8d 100644
> --- a/scripts/include/linux/string.h
> +++ b/scripts/include/linux/string.h
> @@ -16,4 +16,4 @@ int strtobool(const char *s, bool *res);
>  extern size_t strlcpy(char *dest, const char *src, size_t size);
>  #endif
>  
> -#endif /* _LINUX_STRING_H_ */
> +#endif /* _TOOLS_LINUX_STRING_H_ */
> diff --git a/scripts/include/string_util.h b/scripts/include/string_util.h
> new file mode 100644
> index 0000000000..e71aa60d26
> --- /dev/null
> +++ b/scripts/include/string_util.h
> @@ -0,0 +1,65 @@
> +#ifndef _TOOLS_STRING_UTIL_H_
> +#define _TOOLS_STRING_UTIL_H_
> +
> +#include <linux/types.h>
> +#include <stddef.h>
> +
> +// SPDX-SnippetBegin
> +// SPDX-Snippet-Comment: Origin-URL: https://git.pengutronix.de/cgit/barebox/tree/lib/string.c?id=dfcf686f94a5a5387660f2afab79a714baab828a
> +
> +/**
> + * strsep_unescaped - Split a string into tokens, while ignoring escaped delimiters
> + * @s: The string to be searched
> + * @ct: The delimiter characters to search for
> + * @delim: optional pointer to store found delimiter into
> + *
> + * strsep_unescaped() behaves like strsep unless it meets an escaped delimiter.
> + * In that case, it shifts the string back in memory to overwrite the escape's
> + * backslash then continues the search until an unescaped delimiter is found.
> + *
> + * On end of string, this function returns NULL. As long as a non-NULL
> + * value is returned and @delim is not NULL, the found delimiter will
> + * be stored into *@delim.
> + */
> +static char *strsep_unescaped(char **s, const char *ct, char *delim)
> +{
> +	char *sbegin = *s, *hay;
> +	const char *needle;
> +	size_t shift = 0;
> +
> +	if (sbegin == NULL)
> +		return NULL;
> +
> +	for (hay = sbegin; *hay != '\0'; ++hay) {
> +		*hay = hay[shift];
> +
> +		if (*hay == '\\') {
> +			*hay = hay[++shift];
> +			if (*hay != '\\')
> +				continue;
> +		}
> +
> +		for (needle = ct; *needle != '\0'; ++needle) {
> +			if (*hay == *needle)
> +				goto match;
> +		}
> +	}
> +
> +	*s = NULL;
> +	if (delim)
> +		*delim = '\0';
> +	return sbegin;
> +
> +match:
> +	if (delim)
> +		*delim = *hay;
> +	*hay = '\0';
> +	*s = &hay[shift + 1];
> +
> +	return sbegin;
> +}
> +
> +// SPDX-SnippetEnd
> +
> +
> +#endif /* _TOOLS_STRING_UTIL_H_ */
> diff --git a/scripts/keytoc.c b/scripts/keytoc.c
> index 77ada3af45..ed091285f4 100644
> --- a/scripts/keytoc.c
> +++ b/scripts/keytoc.c
> @@ -6,9 +6,12 @@
>   * URI to a C struct suitable to compile with barebox.
>   *
>   * TODO: Find a better way for reimport_key()
> - *
>   */
> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
> +
> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */
> +
> +#include "include/string_util.h"
> +
>  #include <stdio.h>
>  #include <string.h>
>  #include <time.h>
> @@ -784,8 +787,13 @@ static bool parse_info(char *p, struct keyinfo *out)
>  	}
>  }
>  
> -static bool get_name_path(const char *keyspec, struct keyinfo *out)
> +static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
>  {
> +	if (!strncmp(keyspec, "pkcs11:", 7)) { /* legacy format of pkcs11 URI */
> +		out->path = strdup(keyspec);
> +		return true;
> +	}
> +
>  	char *sep, *spec;
>  
>  	spec = strdup(keyspec);
> @@ -814,10 +822,10 @@ static bool get_name_path(const char *keyspec, struct keyinfo *out)
>  
>  int main(int argc, char *argv[])
>  {
> -	int i, opt, ret;
> +	int argi, opt, ret;

The diff gets quite hard to read, if you do such changes in between.
Please reduce the changes per patch to the bare minimum. Renaming could
be done later on in a separate patch.

>  	char *outfile = NULL;
> -	int keycount;
> -	struct keyinfo *keylist;
> +	size_t keycount, num_positionals;
> +	struct keyinfo *keylist = NULL;
>  
>  	outfilep = stdout;
>  
> @@ -852,22 +860,41 @@ int main(int argc, char *argv[])
>  		exit(1);
>  	}
>  
> -	keycount = argc - optind;
> -	keylist = calloc(sizeof(struct keyinfo), keycount);
>  
> -	for (i = 0; i < keycount; i++) {
> -		const char *keyspec = try_resolve_env(argv[optind + i]);
> -		struct keyinfo *info = &keylist[i];
> +	num_positionals = argc - optind;
> +	keycount = num_positionals;
>  
> -		if (!keyspec)
> -			exit(1);
> +	keylist = calloc(keycount, sizeof(*keylist));

Same here, why can't you keep the keycount and keylist handling
unchanged?

> +
> +	if (!keylist)
> +		enomem_exit("push");

This would go into separate patch as well.

As said above, please split your patchset into multiple smaller patches
to make it easier to read.

Regards,
  Marco


> +
> +	int listi = 0;
> +
> +	for (argi = 0; argi < num_positionals; argi++) {
> +		char *arg = strdup(argv[optind + argi]);
> +		char *resolved = try_resolve_env(arg);
>  
> -		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
> -			info->path = strdup(keyspec);
> +		if (arg == resolved) {
> +			keylist[listi].path = arg;
> +			listi++;
>  		} else {
> -			if (!get_name_path(keyspec, info)) {
> -				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
> -				exit(1);
> +			char *keyspecs = strdup(resolved);
> +			char *keyspec;
> +
> +			keycount--;
> +			while ((keyspec = strsep_unescaped(&keyspecs, " ", NULL))) {
> +				keycount++;
> +				keylist = reallocarray(keylist, keycount, sizeof(*keylist));
> +				if (!keylist)
> +					enomem_exit("realloc keylist");
> +				bzero(keylist + (keycount - 1), sizeof(*keylist));
> +				if (!parse_keyspec(keyspec, &keylist[listi])) {
> +					fprintf(stderr, "invalid keyspec %i: %s\n", optind,
> +						keyspec);
> +					exit(1);
> +				}
> +				listi++;
>  			}
>  		}
>  	}
> @@ -885,14 +912,14 @@ int main(int argc, char *argv[])
>  	}
>  
>  
> -	for (i = 0; i < keycount; i++) {
> -		struct keyinfo *info = &keylist[i];
> +	for (argi = 0; argi < keycount; argi++) {
> +		struct keyinfo *info = &keylist[argi];
>  
>  		/* resolve __ENV__ for name_hint and path */
>  		info->name_hint = try_resolve_env(info->name_hint);
>  		info->path = try_resolve_env(info->path);
>  
> -		if (asprintf(&info->name_c, "key_%i", i + 1) < 0)
> +		if (asprintf(&info->name_c, "key_%i", argi + 1) < 0)
>  			enomem_exit("asprintf");
>  
>  		/* unfortunately, the fit name hint is mandatory in the barebox codebase */
> @@ -901,7 +928,7 @@ int main(int argc, char *argv[])
>  
>  		if (!info->keyring) {
>  			info->keyring = strdup("fit");
> -			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + i]);
> +			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + argi]);
>  		}
>  
>  		ret = gen_key(info);
> 
> -- 
> 2.51.2.535.g419c72cb8a
> 
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v2 1/2] crypto: keytoc: Split env-provided full keyspec on spaces

[Jonas Rebmann]

Hi Marco

On 2026-02-18 13:55, Marco Felsch wrote:
> On 26-02-18, Jonas Rebmann wrote:
>> keytoc/CONFIG_CRYPTO_PUBLIC_KEYS can work with a complete keyspec
>> provided by an environment variable as opposed to providing single URIs.
>> This would be a very useful feature if it could also provide any number
>> of keys. Kconfig however provides keytoc with regular keyspecs already
>> split at spaces so without furhter measures, the env variable can only
>> be expanded into a single key.
>>
>> If a complete argument is provided via __ENV, split it at any space
>> character that is not escaped with a backslash in front of it.  An
>> actual backslash in a path needs to be escape with another backslash.
>>
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>>   scripts/include/linux/string.h |  2 +-
>>   scripts/include/string_util.h  | 65 ++++++++++++++++++++++++++++++++++++++
>>   scripts/keytoc.c               | 71 +++++++++++++++++++++++++++++-------------
> 
> Porting strsep_unescaped should be one patch and the keytoc changes
> should be one patch.

I will send a v3 with that change.

>>   3 files changed, 115 insertions(+), 23 deletions(-)
>>
>> diff --git a/scripts/include/linux/string.h b/scripts/include/linux/string.h
>> index 649287b80a..ac06f96d8d 100644
>> --- a/scripts/include/linux/string.h
>> +++ b/scripts/include/linux/string.h
>> @@ -16,4 +16,4 @@ int strtobool(const char *s, bool *res);
>>   extern size_t strlcpy(char *dest, const char *src, size_t size);
>>   #endif
>>   
>> -#endif /* _LINUX_STRING_H_ */
>> +#endif /* _TOOLS_LINUX_STRING_H_ */
>> diff --git a/scripts/include/string_util.h b/scripts/include/string_util.h
>> new file mode 100644
>> index 0000000000..e71aa60d26
>> --- /dev/null
>> +++ b/scripts/include/string_util.h
>> @@ -0,0 +1,65 @@
>> +#ifndef _TOOLS_STRING_UTIL_H_
>> +#define _TOOLS_STRING_UTIL_H_
>> +
>> +#include <linux/types.h>
>> +#include <stddef.h>
>> +
>> +// SPDX-SnippetBegin
>> +// SPDX-Snippet-Comment: Origin-URL: https://git.pengutronix.de/cgit/barebox/tree/lib/string.c?id=dfcf686f94a5a5387660f2afab79a714baab828a
>> +
>> +/**
>> + * strsep_unescaped - Split a string into tokens, while ignoring escaped delimiters
>> + * @s: The string to be searched
>> + * @ct: The delimiter characters to search for
>> + * @delim: optional pointer to store found delimiter into
>> + *
>> + * strsep_unescaped() behaves like strsep unless it meets an escaped delimiter.
>> + * In that case, it shifts the string back in memory to overwrite the escape's
>> + * backslash then continues the search until an unescaped delimiter is found.
>> + *
>> + * On end of string, this function returns NULL. As long as a non-NULL
>> + * value is returned and @delim is not NULL, the found delimiter will
>> + * be stored into *@delim.
>> + */
>> +static char *strsep_unescaped(char **s, const char *ct, char *delim)
>> +{
>> +	char *sbegin = *s, *hay;
>> +	const char *needle;
>> +	size_t shift = 0;
>> +
>> +	if (sbegin == NULL)
>> +		return NULL;
>> +
>> +	for (hay = sbegin; *hay != '\0'; ++hay) {
>> +		*hay = hay[shift];
>> +
>> +		if (*hay == '\\') {
>> +			*hay = hay[++shift];
>> +			if (*hay != '\\')
>> +				continue;
>> +		}
>> +
>> +		for (needle = ct; *needle != '\0'; ++needle) {
>> +			if (*hay == *needle)
>> +				goto match;
>> +		}
>> +	}
>> +
>> +	*s = NULL;
>> +	if (delim)
>> +		*delim = '\0';
>> +	return sbegin;
>> +
>> +match:
>> +	if (delim)
>> +		*delim = *hay;
>> +	*hay = '\0';
>> +	*s = &hay[shift + 1];
>> +
>> +	return sbegin;
>> +}
>> +
>> +// SPDX-SnippetEnd
>> +
>> +
>> +#endif /* _TOOLS_STRING_UTIL_H_ */
>> diff --git a/scripts/keytoc.c b/scripts/keytoc.c
>> index 77ada3af45..ed091285f4 100644
>> --- a/scripts/keytoc.c
>> +++ b/scripts/keytoc.c
>> @@ -6,9 +6,12 @@
>>    * URI to a C struct suitable to compile with barebox.
>>    *
>>    * TODO: Find a better way for reimport_key()
>> - *
>>    */
>> -#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>> +
>> +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" /* ENGINE deprecated in OpenSSL 3.0 */

This line is admittedly unrelated to what I did but I think our
convention here is to rather slip such tiny things into whereever they
come up than to create an extra patch for it. Correct me if I'm wrong.

>> +
>> +#include "include/string_util.h"
>> +
>>   #include <stdio.h>
>>   #include <string.h>
>>   #include <time.h>
>> @@ -784,8 +787,13 @@ static bool parse_info(char *p, struct keyinfo *out)
>>   	}
>>   }
>>   
>> -static bool get_name_path(const char *keyspec, struct keyinfo *out)
>> +static bool parse_keyspec(const char *keyspec, struct keyinfo *out)
>>   {
>> +	if (!strncmp(keyspec, "pkcs11:", 7)) { /* legacy format of pkcs11 URI */
>> +		out->path = strdup(keyspec);
>> +		return true;
>> +	}
>> +
>>   	char *sep, *spec;
>>   
>>   	spec = strdup(keyspec);
>> @@ -814,10 +822,10 @@ static bool get_name_path(const char *keyspec, struct keyinfo *out)
>>   
>>   int main(int argc, char *argv[])
>>   {
>> -	int i, opt, ret;
>> +	int argi, opt, ret;
> 
> The diff gets quite hard to read, if you do such changes in between.
> Please reduce the changes per patch to the bare minimum. Renaming could
> be done later on in a separate patch.

The rename is part of the same change. Since environment variables can
now be expanded into any number of new keyspecs we're moving from
looping arguments as keys to a nested loop over each arguments
expansions set of multiple keys. i is a fine name for an index in a loop
as long as there's only one index and one loop. Replacing that simple
loop with a nested loop is the core of the change I'm making and the
rename from i to argi is part of that.

>>   	char *outfile = NULL;
>> -	int keycount;
>> -	struct keyinfo *keylist;
>> +	size_t keycount, num_positionals;
>> +	struct keyinfo *keylist = NULL;
>>   
>>   	outfilep = stdout;
>>   
>> @@ -852,22 +860,41 @@ int main(int argc, char *argv[])
>>   		exit(1);
>>   	}
>>   
>> -	keycount = argc - optind;
>> -	keylist = calloc(sizeof(struct keyinfo), keycount);
>>   
>> -	for (i = 0; i < keycount; i++) {
>> -		const char *keyspec = try_resolve_env(argv[optind + i]);
>> -		struct keyinfo *info = &keylist[i];
>> +	num_positionals = argc - optind;
>> +	keycount = num_positionals;
>>   
>> -		if (!keyspec)
>> -			exit(1);
>> +	keylist = calloc(keycount, sizeof(*keylist));
> 
> Same here, why can't you keep the keycount and keylist handling
> unchanged?

The same change: As a direct consequence of expanding multiple keys from
one environment variable, we cannot just assume that the keylist length
is the number of positional arguments, we cannot know the number of keys
we end up with in advance and hence we need to dynamically resize the
keylist.

>> +
>> +	if (!keylist)
>> +		enomem_exit("push");
> 
> This would go into separate patch as well.
> 
> As said above, please split your patchset into multiple smaller patches
> to make it easier to read.

Those changes to keytoc are mostly one single atomic change that cannot
be split into more patches. This includes new memory allocations to
handle growth of the keylist.

> Regards,
>    Marco
> 
> 
>> +
>> +	int listi = 0;
>> +
>> +	for (argi = 0; argi < num_positionals; argi++) {
>> +		char *arg = strdup(argv[optind + argi]);
>> +		char *resolved = try_resolve_env(arg);
>>   
>> -		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
>> -			info->path = strdup(keyspec);
>> +		if (arg == resolved) {
>> +			keylist[listi].path = arg;
>> +			listi++;
>>   		} else {
>> -			if (!get_name_path(keyspec, info)) {
>> -				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
>> -				exit(1);
>> +			char *keyspecs = strdup(resolved);
>> +			char *keyspec;
>> +
>> +			keycount--;
>> +			while ((keyspec = strsep_unescaped(&keyspecs, " ", NULL))) {
>> +				keycount++;
>> +				keylist = reallocarray(keylist, keycount, sizeof(*keylist));
>> +				if (!keylist)
>> +					enomem_exit("realloc keylist");
>> +				bzero(keylist + (keycount - 1), sizeof(*keylist));
>> +				if (!parse_keyspec(keyspec, &keylist[listi])) {
>> +					fprintf(stderr, "invalid keyspec %i: %s\n", optind,
>> +						keyspec);
>> +					exit(1);
>> +				}
>> +				listi++;
>>   			}
>>   		}
>>   	}
>> @@ -885,14 +912,14 @@ int main(int argc, char *argv[])
>>   	}
>>   
>>   
>> -	for (i = 0; i < keycount; i++) {
>> -		struct keyinfo *info = &keylist[i];
>> +	for (argi = 0; argi < keycount; argi++) {
>> +		struct keyinfo *info = &keylist[argi];
>>   
>>   		/* resolve __ENV__ for name_hint and path */
>>   		info->name_hint = try_resolve_env(info->name_hint);
>>   		info->path = try_resolve_env(info->path);
>>   
>> -		if (asprintf(&info->name_c, "key_%i", i + 1) < 0)
>> +		if (asprintf(&info->name_c, "key_%i", argi + 1) < 0)
>>   			enomem_exit("asprintf");
>>   
>>   		/* unfortunately, the fit name hint is mandatory in the barebox codebase */
>> @@ -901,7 +928,7 @@ int main(int argc, char *argv[])
>>   
>>   		if (!info->keyring) {
>>   			info->keyring = strdup("fit");
>> -			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + i]);
>> +			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + argi]);
>>   		}
>>   
>>   		ret = gen_key(info);
>>
>> -- 
>> 2.51.2.535.g419c72cb8a
>>
>>
>>
> 

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

[PATCH v2 2/2] Documentation: migration-guides: Document change in keyspec env vars

[Jonas Rebmann]

Only users providing complete keyspecs containing backslashes or spaces
via an environment variable are affected by this change in the handling
of keytoc's command line arguments.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 Documentation/migration-guides/migration-master.rst | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/migration-guides/migration-master.rst b/Documentation/migration-guides/migration-master.rst
index 28d418dee1..97e7b7173b 100644
--- a/Documentation/migration-guides/migration-master.rst
+++ b/Documentation/migration-guides/migration-master.rst
@@ -1,5 +1,22 @@
 :orphan:
 
+CONFIG_CRYPTO_PUBLIC_KEYS
+-------------------------
+
+The syntax of keytoc keyspecs when fully provided via an environment variable
+was slightly changed to allow any number of keyspecs to be provided via an
+environment variable. Such environment variables are now split at spaces to be
+interpreted as multiple keyspecs. Any literal spaces and backslashes contained
+in such keyspecs need to be escaped with a backslash.
+
+This only applies to the form:
+
+  CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__A"
+
+While the interpretation of environment variables specifying hint or URI remains unchanged:
+
+  CONFIG_CRYPTO_PUBLIC_KEYS="keyring=kr:__ENV__B"
+
 ARM NXP i.MX8MP
 ---------------
 

-- 
2.51.2.535.g419c72cb8a