[PATCH] image-fit: don't set compression if it can't be read

fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org --- boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 86dbfbcfed..b659825305 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1024,10 +1024,8 @@ static int bootm_host_load_image(const void *fit, int req_image_type, return -EINVAL; }
- if (fit_image_get_comp(fit, noffset, &imape_comp)) { - puts("Can't get image compression!\n"); - return -EINVAL; - } + if (fit_image_get_comp(fit, noffset, &imape_comp)) + image_comp = IH_COMP_NONE;
/* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4; diff --git a/boot/image-fit.c b/boot/image-fit.c index df3e5df883..21dbd05118 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -477,7 +477,7 @@ void fit_print_contents(const void *fit) void fit_image_print(const void *fit, int image_noffset, const char *p) { char *desc; - uint8_t type, arch, os, comp; + uint8_t type, arch, os, comp = IH_COMP_NONE; size_t size; ulong load, entry; const void *data; @@ -794,7 +794,6 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) data = fdt_getprop(fit, noffset, FIT_COMP_PROP, &len); if (data == NULL) { fit_get_debug(fit, noffset, FIT_COMP_PROP, len); - *comp = -1; return -1; }
diff --git a/cmd/ximg.c b/cmd/ximg.c index 65ba41320a..f84141ff45 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -171,11 +171,8 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }
- if (fit_image_get_comp(fit_hdr, noffset, &comp)) { - puts("Could not find script subimage " - "compression type\n"); - return 1; - } + if (fit_image_get_comp(fit_hdr, noffset, &comp)) + comp = IH_COMP_NONE;
data = (ulong)fit_data; len = (ulong)fit_len;

On Mon, 15 Aug 2022 at 04:39, Daniel Golle daniel@makrotopia.org wrote:
fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org
boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Mon, Aug 15, 2022 at 12:38:27PM +0200, Daniel Golle wrote:
fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org Reviewed-by: Simon Glass sjg@chromium.org
This causes most platforms to fail to build with an error such as: https://source.denx.de/u-boot/u-boot/-/jobs/486959#L140

fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org --- v2: fix typo 'imape_comp' vs. 'image_comp' boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 86dbfbcfed..fcdf23f19c 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1024,10 +1024,8 @@ static int bootm_host_load_image(const void *fit, int req_image_type, return -EINVAL; }
- if (fit_image_get_comp(fit, noffset, &imape_comp)) { - puts("Can't get image compression!\n"); - return -EINVAL; - } + if (fit_image_get_comp(fit, noffset, &imape_comp)) + imape_comp = IH_COMP_NONE;
/* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4; diff --git a/boot/image-fit.c b/boot/image-fit.c index df3e5df883..21dbd05118 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -477,7 +477,7 @@ void fit_print_contents(const void *fit) void fit_image_print(const void *fit, int image_noffset, const char *p) { char *desc; - uint8_t type, arch, os, comp; + uint8_t type, arch, os, comp = IH_COMP_NONE; size_t size; ulong load, entry; const void *data; @@ -794,7 +794,6 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) data = fdt_getprop(fit, noffset, FIT_COMP_PROP, &len); if (data == NULL) { fit_get_debug(fit, noffset, FIT_COMP_PROP, len); - *comp = -1; return -1; }
diff --git a/cmd/ximg.c b/cmd/ximg.c index 65ba41320a..f84141ff45 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -171,11 +171,8 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }
- if (fit_image_get_comp(fit_hdr, noffset, &comp)) { - puts("Could not find script subimage " - "compression type\n"); - return 1; - } + if (fit_image_get_comp(fit_hdr, noffset, &comp)) + comp = IH_COMP_NONE;
data = (ulong)fit_data; len = (ulong)fit_len;

Hi Daniel,
On Fri, 26 Aug 2022 at 15:28, Daniel Golle daniel@makrotopia.org wrote:
fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org
v2: fix typo 'imape_comp' vs. 'image_comp' boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 86dbfbcfed..fcdf23f19c 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1024,10 +1024,8 @@ static int bootm_host_load_image(const void *fit, int req_image_type, return -EINVAL; }
if (fit_image_get_comp(fit, noffset, &imape_comp)) {
puts("Can't get image compression!\n");
return -EINVAL;
}
if (fit_image_get_comp(fit, noffset, &imape_comp))
imape_comp = IH_COMP_NONE; /* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4;
diff --git a/boot/image-fit.c b/boot/image-fit.c index df3e5df883..21dbd05118 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -477,7 +477,7 @@ void fit_print_contents(const void *fit) void fit_image_print(const void *fit, int image_noffset, const char *p) { char *desc;
uint8_t type, arch, os, comp;
uint8_t type, arch, os, comp = IH_COMP_NONE; size_t size; ulong load, entry; const void *data;
@@ -794,7 +794,6 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) data = fdt_getprop(fit, noffset, FIT_COMP_PROP, &len); if (data == NULL) { fit_get_debug(fit, noffset, FIT_COMP_PROP, len);
*comp = -1; return -1; }
diff --git a/cmd/ximg.c b/cmd/ximg.c index 65ba41320a..f84141ff45 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -171,11 +171,8 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }
if (fit_image_get_comp(fit_hdr, noffset, &comp)) {
puts("Could not find script subimage "
"compression type\n");
return 1;
}
if (fit_image_get_comp(fit_hdr, noffset, &comp))
comp = IH_COMP_NONE; data = (ulong)fit_data; len = (ulong)fit_len;
-- 2.37.2
Well it should be imape_comp, so could you fix that error first (in a patch before this one)?
Regards, SImon

Chage variable name 'imape_comp' to the supposedly intended name 'image_comp'.
Signed-off-by: Daniel Golle daniel@makrotopia.org --- boot/bootm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 86dbfbcfed..4de573e24e 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1006,7 +1006,7 @@ static int bootm_host_load_image(const void *fit, int req_image_type, int noffset; ulong load_end, buf_size; uint8_t image_type; - uint8_t imape_comp; + uint8_t image_comp; void *load_buf; int ret;
@@ -1032,12 +1032,12 @@ static int bootm_host_load_image(const void *fit, int req_image_type, /* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4; load_buf = malloc(buf_size); - ret = image_decomp(imape_comp, 0, data, image_type, load_buf, + ret = image_decomp(image_comp, 0, data, image_type, load_buf, (void *)data, len, buf_size, &load_end); free(load_buf);
if (ret) { - ret = handle_decomp_error(imape_comp, load_end - 0, buf_size, ret); + ret = handle_decomp_error(image_comp, load_end - 0, buf_size, ret); if (ret != BOOTM_ERR_UNIMPLEMENTED) return ret; }

fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org --- boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 4de573e24e..29c067fae7 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1024,10 +1024,8 @@ static int bootm_host_load_image(const void *fit, int req_image_type, return -EINVAL; }
- if (fit_image_get_comp(fit, noffset, &imape_comp)) { - puts("Can't get image compression!\n"); - return -EINVAL; - } + if (fit_image_get_comp(fit, noffset, &image_comp)) + image_comp = IH_COMP_NONE;
/* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4; diff --git a/boot/image-fit.c b/boot/image-fit.c index df3e5df883..21dbd05118 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -477,7 +477,7 @@ void fit_print_contents(const void *fit) void fit_image_print(const void *fit, int image_noffset, const char *p) { char *desc; - uint8_t type, arch, os, comp; + uint8_t type, arch, os, comp = IH_COMP_NONE; size_t size; ulong load, entry; const void *data; @@ -794,7 +794,6 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) data = fdt_getprop(fit, noffset, FIT_COMP_PROP, &len); if (data == NULL) { fit_get_debug(fit, noffset, FIT_COMP_PROP, len); - *comp = -1; return -1; }
diff --git a/cmd/ximg.c b/cmd/ximg.c index 65ba41320a..f84141ff45 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -171,11 +171,8 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }
- if (fit_image_get_comp(fit_hdr, noffset, &comp)) { - puts("Could not find script subimage " - "compression type\n"); - return 1; - } + if (fit_image_get_comp(fit_hdr, noffset, &comp)) + comp = IH_COMP_NONE;
data = (ulong)fit_data; len = (ulong)fit_len;

Chage variable name 'imape_comp' to the supposedly intended name 'image_comp'.
Signed-off-by: Daniel Golle daniel@makrotopia.org --- v4: add missing name replacement
boot/bootm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 86dbfbcfed..63c79a9cfc 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1006,7 +1006,7 @@ static int bootm_host_load_image(const void *fit, int req_image_type, int noffset; ulong load_end, buf_size; uint8_t image_type; - uint8_t imape_comp; + uint8_t image_comp; void *load_buf; int ret;
@@ -1024,7 +1024,7 @@ static int bootm_host_load_image(const void *fit, int req_image_type, return -EINVAL; }
- if (fit_image_get_comp(fit, noffset, &imape_comp)) { + if (fit_image_get_comp(fit, noffset, &image_comp)) { puts("Can't get image compression!\n"); return -EINVAL; } @@ -1032,12 +1032,12 @@ static int bootm_host_load_image(const void *fit, int req_image_type, /* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4; load_buf = malloc(buf_size); - ret = image_decomp(imape_comp, 0, data, image_type, load_buf, + ret = image_decomp(image_comp, 0, data, image_type, load_buf, (void *)data, len, buf_size, &load_end); free(load_buf);
if (ret) { - ret = handle_decomp_error(imape_comp, load_end - 0, buf_size, ret); + ret = handle_decomp_error(image_comp, load_end - 0, buf_size, ret); if (ret != BOOTM_ERR_UNIMPLEMENTED) return ret; }

On Fri, 26 Aug 2022 at 21:15, Daniel Golle daniel@makrotopia.org wrote:
Chage variable name 'imape_comp' to the supposedly intended name
Change
'image_comp'.
Signed-off-by: Daniel Golle daniel@makrotopia.org
v4: add missing name replacement
boot/bootm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Mon, Aug 29, 2022 at 08:30:14PM -0600, Simon Glass wrote:
On Fri, 26 Aug 2022 at 21:15, Daniel Golle daniel@makrotopia.org wrote:
Chage variable name 'imape_comp' to the supposedly intended name
Change
Can you fix that on-the-fly while comitting or should I repost with that typo in the patch description fixed?
'image_comp'.
Signed-off-by: Daniel Golle daniel@makrotopia.org
v4: add missing name replacement
boot/bootm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Daniel,
On Mon, 29 Aug 2022 at 21:04, Daniel Golle daniel@makrotopia.org wrote:
On Mon, Aug 29, 2022 at 08:30:14PM -0600, Simon Glass wrote:
On Fri, 26 Aug 2022 at 21:15, Daniel Golle daniel@makrotopia.org wrote:
Chage variable name 'imape_comp' to the supposedly intended name
Change
Can you fix that on-the-fly while comitting or should I repost with that typo in the patch description fixed?
Yes I suspect Tom can do that.
Regards, Simon
'image_comp'.
Signed-off-by: Daniel Golle daniel@makrotopia.org
v4: add missing name replacement
boot/bootm.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Tue, Aug 30, 2022 at 09:56:56AM -0600, Simon Glass wrote:
Hi Daniel,
On Mon, 29 Aug 2022 at 21:04, Daniel Golle daniel@makrotopia.org wrote:
On Mon, Aug 29, 2022 at 08:30:14PM -0600, Simon Glass wrote:
On Fri, 26 Aug 2022 at 21:15, Daniel Golle daniel@makrotopia.org wrote:
Chage variable name 'imape_comp' to the supposedly intended name
Change
Can you fix that on-the-fly while comitting or should I repost with that typo in the patch description fixed?
Yes I suspect Tom can do that.
Yes, that's fine.

On Sat, Aug 27, 2022 at 04:14:42AM +0100, Daniel Golle wrote:
Change variable name 'imape_comp' to the supposedly intended name 'image_comp'.
Signed-off-by: Daniel Golle daniel@makrotopia.org Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org --- v2: fix typo 'imape_comp' vs. 'image_comp' v3: rather fix the typo everywhere in an additional patch before v4: rebase on updated patch fixing typo
boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c index 63c79a9cfc..29c067fae7 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -1024,10 +1024,8 @@ static int bootm_host_load_image(const void *fit, int req_image_type, return -EINVAL; }
- if (fit_image_get_comp(fit, noffset, &image_comp)) { - puts("Can't get image compression!\n"); - return -EINVAL; - } + if (fit_image_get_comp(fit, noffset, &image_comp)) + image_comp = IH_COMP_NONE;
/* Allow the image to expand by a factor of 4, should be safe */ buf_size = (1 << 20) + len * 4; diff --git a/boot/image-fit.c b/boot/image-fit.c index df3e5df883..21dbd05118 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -477,7 +477,7 @@ void fit_print_contents(const void *fit) void fit_image_print(const void *fit, int image_noffset, const char *p) { char *desc; - uint8_t type, arch, os, comp; + uint8_t type, arch, os, comp = IH_COMP_NONE; size_t size; ulong load, entry; const void *data; @@ -794,7 +794,6 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) data = fdt_getprop(fit, noffset, FIT_COMP_PROP, &len); if (data == NULL) { fit_get_debug(fit, noffset, FIT_COMP_PROP, len); - *comp = -1; return -1; }
diff --git a/cmd/ximg.c b/cmd/ximg.c index 65ba41320a..f84141ff45 100644 --- a/cmd/ximg.c +++ b/cmd/ximg.c @@ -171,11 +171,8 @@ do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return 1; }
- if (fit_image_get_comp(fit_hdr, noffset, &comp)) { - puts("Could not find script subimage " - "compression type\n"); - return 1; - } + if (fit_image_get_comp(fit_hdr, noffset, &comp)) + comp = IH_COMP_NONE;
data = (ulong)fit_data; len = (ulong)fit_len;

On Fri, 26 Aug 2022 at 21:18, Daniel Golle daniel@makrotopia.org wrote:
fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org
v2: fix typo 'imape_comp' vs. 'image_comp' v3: rather fix the typo everywhere in an additional patch before v4: rebase on updated patch fixing typo
boot/bootm.c | 6 ++---- boot/image-fit.c | 3 +-- cmd/ximg.c | 7 ++----- 3 files changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Aug 27, 2022 at 04:17:28AM +0100, Daniel Golle wrote:
fit_image_get_comp() should not set value -1 in case it can't read the compression node. Instead, leave the value untouched in that case as it can be absent and a default value previously defined by the caller of fit_image_get_comp() should be used.
As a result the warning message WARNING: 'compression' nodes for ramdisks are deprecated, please fix your .its file! no longer shows if the compression node is actually absent.
Signed-off-by: Daniel Golle daniel@makrotopia.org Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
Daniel Golle
-
Simon Glass
-
Tom Rini