[PATCH] commands: saveenv: mention where environment is saved to

[PATCH] commands: saveenv: mention where environment is saved to

[Ahmad Fatoum]

We already mention it on loadenv, but it makes sense to do it on saveenv
too to assist with debugging.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/saveenv.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/commands/saveenv.c b/commands/saveenv.c
index 203729efbb1a..d8aeb31f051c 100644
--- a/commands/saveenv.c
+++ b/commands/saveenv.c
@@ -13,9 +13,8 @@ static int do_saveenv(int argc, char *argv[])
 {
 	int ret, opt;
 	unsigned envfs_flags = 0;
-	char *filename = NULL, *dirname = NULL;
+	const char *filename = NULL, *dirname = NULL;
 
-	printf("saving environment\n");
 	while ((opt = getopt(argc, argv, "z")) > 0) {
 		switch (opt) {
 		case 'z':
@@ -33,6 +32,12 @@ static int do_saveenv(int argc, char *argv[])
 	/* destination only given? */
 	if (argc - optind > 0)
 		filename = argv[optind];
+	if (!filename)
+		filename = default_environment_path_get();
+	if (!filename)
+		return -ENOENT;
+
+	printf("saving environment to %s\n", filename);
 
 	ret = envfs_save(filename, dirname, envfs_flags);
 
-- 
2.39.5

Re: [PATCH] commands: saveenv: mention where environment is saved to

[Sascha Hauer]

On Mon, Apr 14, 2025 at 08:24:19AM +0200, Ahmad Fatoum wrote:
> We already mention it on loadenv, but it makes sense to do it on saveenv
> too to assist with debugging.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  commands/saveenv.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/commands/saveenv.c b/commands/saveenv.c
> index 203729efbb1a..d8aeb31f051c 100644
> --- a/commands/saveenv.c
> +++ b/commands/saveenv.c
> @@ -13,9 +13,8 @@ static int do_saveenv(int argc, char *argv[])
>  {
>  	int ret, opt;
>  	unsigned envfs_flags = 0;
> -	char *filename = NULL, *dirname = NULL;
> +	const char *filename = NULL, *dirname = NULL;
>  
> -	printf("saving environment\n");
>  	while ((opt = getopt(argc, argv, "z")) > 0) {
>  		switch (opt) {
>  		case 'z':
> @@ -33,6 +32,12 @@ static int do_saveenv(int argc, char *argv[])
>  	/* destination only given? */
>  	if (argc - optind > 0)
>  		filename = argv[optind];
> +	if (!filename)
> +		filename = default_environment_path_get();
> +	if (!filename)
> +		return -ENOENT;

default_environment_path_get() will never return a NULL pointer unless
the static inline wrapper is used. The saveenv command depends on
CONFIG_ENV_HANDLING, so when won't get here with CONFIG_ENV_HANDLING
disabled.

I think we can safely remove this last check, or if we think we need it,
then we should have an error message here as well.

> +
> +	printf("saving environment to %s\n", filename);
>  
>  	ret = envfs_save(filename, dirname, envfs_flags);

Now that we always pass a filename to envfs_save(), should we remove
the check for !filename from the function?

Sascha

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