Hello!
I would suggest to change description of the patch from
"mvebu: fix end-of-array check"
to something like:
"arm: mvebu: Espressobin: fix end-of-array check in env"
as it affects only Espressobin boards (not other mvebu).
Stefan, please look below as this issue/fix is important.
On Wednesday 30 November 2022 13:33:40 Derek LaHousse wrote:
> Properly seek the end of default_environment variables.
>
> The current algorithm overwrites from the second variable. This
> replacement finds the end of the array of strings.
>
> Stomped variables include "board", "soc", "loadaddr". These can be
> seen on a "env default -a" after patch, but they are not seen with a
> version before the patch.
This is a real issue which I introduced in the past. So it some fix for
this issue should be pulled into the v2023.01 release.
> Signed-off-by: Derek LaHousse <de...@seaofdirac.org>
> ---
> board/Marvell/mvebu_armada-37xx/board.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c
> b/board/Marvell/mvebu_armada-37xx/board.c
> index c6ecc323bb..ac29ac5b95 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -100,8 +100,11 @@ int board_late_init(void)
> return 0;
>
> /* Find free buffer in default_environment[] for new variables
> */
> - while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
> - ptr += 2;
> + if (*ptr != '\0') { // Defending against empty default env
> + while ((i = strlen(ptr)) != 0) {
> + ptr += i + 1;
> + }
> + }
If I'm looking at the outer-if condition correctly then it is not
needed. strlen("") returns zero and so inner-while loop stops
immediately.
My proposed fix for this issue was just changing correct while loop
check to ensure that ptr is set after the _last_ variable.
- while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
- ptr += 2;
+ while (*ptr != '\0' || *(ptr+1) != '\0') ptr++;
+ ptr++;
Both changes should be equivalent, but I'm not sure which one is more
readable. The original issue was introduced really by non-readable
code...
Stefan, do you have a preference which one fix is better / more
readable?
It would be really a good idea if you try to check if any of those
proposed fixes/patches are now really correct.
>
> /*
> * Ensure that 'env default -a' does not erase permanent MAC
> addresses
> --
> 2.30.2
>
>
>
