[OSS-Tools] [PATCH v2] barebox-state: have the --set option to avoid writes if possible

[OSS-Tools] [PATCH v2] barebox-state: have the --set option to avoid writes if possible

[Ahmad Fatoum]

barebox-state --set always dirties the state when successful. Users
seeking to conserve write cycles thus have to --get the variable in
question first to check whether to write it. Make life of such users
easier by having barebox-state support this out-of-the-box.

This allows users to fire and forget execute barebox-state.

This arguably should have been the behavior from the beginning,
the state implementation shared by barebox and dt-utils already
marks the state dirty if buckets appear corrupted on probe, so
there is no extra benefit in always executing the write.

The comparison to determine whether the state should be dirtied
does an extra allocation in interest of clarity.
This overhead is deemed negligible compared to I/O and it makes
the code easier to follow.

Suggested-by: Jan Lübbe <jlu@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - incorporate Jan's (off-list) suggestion to just change --set
    behavior. state implementation already dirties state if a
    bucket is corrupt, so there is really no valid use case for
    not conserving writes by default.
---
 src/barebox-state.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/barebox-state.c b/src/barebox-state.c
index cd56ce7192c3..7b5c0dae00dd 100644
--- a/src/barebox-state.c
+++ b/src/barebox-state.c
@@ -283,6 +283,7 @@ static int state_set_var(struct state *state, const char *var, const char *val)
 {
 	struct state_variable *sv;
 	struct variable_str_type *vtype;
+	char *oldval;
 	int ret;
 
 	sv = state_find_var(state, var);
@@ -296,6 +297,14 @@ static int state_set_var(struct state *state, const char *var, const char *val)
 	if (!vtype->set)
 		return -EPERM;
 
+	oldval = vtype->get(sv);
+	if (!IS_ERR(oldval)) {
+		bool equal = strcmp(oldval, val) == 0;
+		free(oldval);
+		if (equal)
+			return 0;
+	}
+
 	ret = vtype->set(sv, val);
 	if (ret)
 		return ret;
-- 
2.28.0


_______________________________________________
OSS-Tools mailing list
OSS-Tools@pengutronix.de

Re: [OSS-Tools] [PATCH v2] barebox-state: have the --set option to avoid writes if possible

[Roland Hieber]

On Fri, Nov 13, 2020 at 08:08:56PM +0100, Ahmad Fatoum wrote:
> barebox-state --set always dirties the state when successful. Users
> seeking to conserve write cycles thus have to --get the variable in
> question first to check whether to write it. Make life of such users
> easier by having barebox-state support this out-of-the-box.
> 
> This allows users to fire and forget execute barebox-state.
> 
> This arguably should have been the behavior from the beginning,
> the state implementation shared by barebox and dt-utils already
> marks the state dirty if buckets appear corrupted on probe, so
> there is no extra benefit in always executing the write.
> 
> The comparison to determine whether the state should be dirtied
> does an extra allocation in interest of clarity.
> This overhead is deemed negligible compared to I/O and it makes
> the code easier to follow.
> 
> Suggested-by: Jan Lübbe <jlu@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> v1 -> v2:
>   - incorporate Jan's (off-list) suggestion to just change --set
>     behavior. state implementation already dirties state if a
>     bucket is corrupt, so there is really no valid use case for
>     not conserving writes by default.
> ---
>  src/barebox-state.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Thanks, applied.

 - Roland

> 
> diff --git a/src/barebox-state.c b/src/barebox-state.c
> index cd56ce7192c3..7b5c0dae00dd 100644
> --- a/src/barebox-state.c
> +++ b/src/barebox-state.c
> @@ -283,6 +283,7 @@ static int state_set_var(struct state *state, const char *var, const char *val)
>  {
>  	struct state_variable *sv;
>  	struct variable_str_type *vtype;
> +	char *oldval;
>  	int ret;
>  
>  	sv = state_find_var(state, var);
> @@ -296,6 +297,14 @@ static int state_set_var(struct state *state, const char *var, const char *val)
>  	if (!vtype->set)
>  		return -EPERM;
>  
> +	oldval = vtype->get(sv);
> +	if (!IS_ERR(oldval)) {
> +		bool equal = strcmp(oldval, val) == 0;
> +		free(oldval);
> +		if (equal)
> +			return 0;
> +	}
> +
>  	ret = vtype->set(sv, val);
>  	if (ret)
>  		return ret;
> -- 
> 2.28.0
> 
> 
> _______________________________________________
> OSS-Tools mailing list
> OSS-Tools@pengutronix.de

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

_______________________________________________
OSS-Tools mailing list
OSS-Tools@pengutronix.de