[meta-swupdate][PATCH] swupdate-lib.bblcass: replace swupdate_get_size with swupdate_get_decrypted_size

[sam.van....@gmail.com]

From: Sam Van Den Berge <sam.van....@gmail.com>

Commit c1586c7de422c introduced swupdate_get_size with the intention to
have the unencrypted size of an artifact. However it gets the size from
the artifact in the "s" folder which is the encrypted artifact.
Hence currently swupdate_get_size gives the size of the encrypted
artifact. To avoid any ambiguity I think it's best to rename that
function to swupdate_get_decrypted_size and to get the file from the
DEPLOY_DIR_IMAGE which contains the unencrypted artifact. Then it can be
used without any ambiguity:

properties: {
decrypted-size = "$swupdate_get_decrypted_size(...)";
}

Fixes: c1586c7de422c ("class: add function to get file size")
Signed-off-by: Sam Van Den Berge <sam.van....@gmail.com>
---
classes/swupdate-lib.bbclass | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass
index 14a2a08..30d2e04 100644
--- a/classes/swupdate-lib.bbclass
+++ b/classes/swupdate-lib.bbclass
@@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename):
m.update(data)
return m.hexdigest()

-def swupdate_get_size(d, s, filename):
+def swupdate_get_decrypted_size(d, s, filename):
import os

- fname = os.path.join(s, filename)
+ deploydir = d.getVar('DEPLOY_DIR_IMAGE', True)
+ fname = os.path.join(deploydir, filename)
fsize = os.path.getsize(fname)
return str(fsize)

--
2.34.1

[Sam Van Den Berge]

Ping.

[Stefano Babic]

Hallo Sam,

added Ayoub as author of the original commit:

On 27.10.22 16:21, sam.van....@gmail.com wrote:
> From: Sam Van Den Berge <sam.van....@gmail.com>
>
> Commit c1586c7de422c introduced swupdate_get_size with the intention to
> have the unencrypted size of an artifact. However it gets the size from
> the artifact in the "s" folder which is the encrypted artifact.

Not sure here. The function simply returns the size of the file passed
as parameter. We can write

size = "$swupdate_get_size(unencrypted_file)";

or

size = "$swupdate_get_size(encrypted_file.enc)";

if you pass the encrypted file, you get the size of the encrypted file.

> Hence currently swupdate_get_size gives the size of the encrypted
> artifact. To avoid any ambiguity I think it's best to rename that
> function to swupdate_get_decrypted_size and to get the file from the
> DEPLOY_DIR_IMAGE which contains the unencrypted artifact.

This is mostly where I stumbled. The logic is that files are already
prepared for any function, and no fix path should be set inside the
function itself.

> Then it can be
> used without any ambiguity:
>
> properties: {
> decrypted-size = "$swupdate_get_decrypted_size(...)";
> }
>
> Fixes: c1586c7de422c ("class: add function to get file size")
> Signed-off-by: Sam Van Den Berge <sam.van....@gmail.com>
> ---
> classes/swupdate-lib.bbclass | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/classes/swupdate-lib.bbclass b/classes/swupdate-lib.bbclass
> index 14a2a08..30d2e04 100644
> --- a/classes/swupdate-lib.bbclass
> +++ b/classes/swupdate-lib.bbclass
> @@ -40,10 +40,11 @@ def swupdate_get_sha256(d, s, filename):
> m.update(data)
> return m.hexdigest()
>
> -def swupdate_get_size(d, s, filename):
> +def swupdate_get_decrypted_size(d, s, filename):
> import os
>
> - fname = os.path.join(s, filename)
> + deploydir = d.getVar('DEPLOY_DIR_IMAGE', True)
> + fname = os.path.join(deploydir, filename)
> fsize = os.path.getsize(fname)
> return str(fsize)
>

Best regards,
Stefano Babic

--
=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

[Stefano Babic]

+Ayoub, this time with the right E-Mail address.

[ayoub...@googlemail.com]

Hello,

Stefano is right you should get the decrypted file size if you want to rename this function.

best

[ayoub...@googlemail.com]

There may be also other use cases to have 'swupdate_get_size' so I'd propose to have a new function 'swupdate_get_decrypted_size'

[Sam Van Den Berge]

On Tue, Nov 08, 2022 at 03:56:59PM +0100, Stefano Babic wrote:
> +Ayoub, this time with the right E-Mail address.
>
> On 08.11.22 14:41, Stefano Babic wrote:
> > Hallo Sam,
> >
> > added Ayoub as author of the original commit:
> >
> > On 27.10.22 16:21, sam.van....@gmail.com wrote:
> > > From: Sam Van Den Berge <sam.van....@gmail.com>
> > >
> > > Commit c1586c7de422c introduced swupdate_get_size with the intention to
> > > have the unencrypted size of an artifact. However it gets the size from
> > > the artifact in the "s" folder which is the encrypted artifact.
> >
> > Not sure here. The function simply returns the size of the file passed
> > as parameter. We can write
> >
> >     size = "$swupdate_get_size(unencrypted_file)";
> >
> > or
> >
> >     size = "$swupdate_get_size(encrypted_file.enc)";
> >
> > if you pass the encrypted file, you get the size of the encrypted file.
> >

As I understand, if you request to encrypt an image via
SWUPDATE_IMAGES_ENCRYPTED, the function add_image_to_swu will encrypt the image
and store it in the destination "S". The unencrypted file is not available in
the "S" folder so you can't get the "decrypted size" anymore via files in the
"S" folder. There is also nowhere a relation anymore to the original source
(unencrypted file).

I might be missing something but with the current implementation I don't
understand how to automatically get the decrypted size of an image. I'm
wondering how it could have worked for Ayoub. Unless by coincidence the
decrypted and encrypted size were the same.

After processing the above feedback I got, I did find one way to get the
decrypted size with the current implementation:

size = "$swupdate_get_size(@@DEPLOY_DIR_IMAGE@@/image)"

In this case, the filename argument in os.path.join(s, filename) in
swupdate_get_size will be an absolute path and so os.path.join will return
filename which is the absolute path to "image". Then I do seem to get the
correct decrypted size. Is this the recommended way of working?

[ayoub...@googlemail.com]

I didn't use it to get the decrypted size !

I was just given as an example, I use it for something different actually.