* Re: [PATCH 1/1] eal: enable xz read support and ignore warning
2023-09-22 16:53 [PATCH 1/1] eal: enable xz read support and ignore warning Srikanth Yalavarthi
@ 2023-09-25 9:10 ` David Marchand
2023-09-26 13:32 ` [EXT] " Srikanth Yalavarthi
2023-09-26 13:30 ` [PATCH v2 1/1] eal: update " Srikanth Yalavarthi
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: David Marchand @ 2023-09-25 9:10 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, dev, sshankarnara, aprabhu, ptakkar,
jerinjacobk, stable
Hello,
Thank you for the patch.
On Fri, Sep 22, 2023 at 6:54 PM Srikanth Yalavarthi
<syalavarthi@marvell.com> wrote:
>
> archive_read_support_filter_xz returns a warning when
> compression is not fully supported and is supported
> through external program. This warning can be ignored
> when reading the files through firmware open as only
> decompression is required.
- I don't understand the last sentence, it seems to state something
about *only* needing decompression support but well,
archive_read_support_filter_xz (like archive_read_support_filter_*
other helpers) *is about* decompressing a file.
- I can't reproduce this ARCHIVE_WARN thing, not sure which libarchive
you use, or which knob/build option triggered this behavior you
observe.
So I need you to to double check how this change affects the code.
Please pass a xz-compressed mldev fw .bin file and confirm it still works.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> ---
> lib/eal/unix/eal_firmware.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
> index d1616b0bd9..05c06c222a 100644
> --- a/lib/eal/unix/eal_firmware.c
> +++ b/lib/eal/unix/eal_firmware.c
> @@ -25,12 +25,19 @@ static int
> firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t blocksize)
> {
> struct archive_entry *e;
> + int err;
>
> ctx->a = archive_read_new();
> if (ctx->a == NULL)
> return -1;
> +
> + err = archive_read_support_filter_xz(ctx->a);
> + if (err != ARCHIVE_OK && err != ARCHIVE_WARN) {
> + ctx->a = NULL;
> + return -1;
> + }
- This patch leaks ctx->a content on error.
Plus I prefer we keep the original order of the code because it
matches what libarchive does: first look for an archive format, then
next look for compression matters.
The simpler is to add an error label like I did in the debug patch.
Something like:
diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
index d1616b0bd9..269688d550 100644
--- a/lib/eal/unix/eal_firmware.c
+++ b/lib/eal/unix/eal_firmware.c
@@ -25,19 +25,27 @@ static int
firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t
blocksize)
{
struct archive_entry *e;
+ int err;
ctx->a = archive_read_new();
if (ctx->a == NULL)
return -1;
- if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
- archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
- archive_read_open_filename(ctx->a, name,
blocksize) != ARCHIVE_OK ||
- archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
- archive_read_free(ctx->a);
- ctx->a = NULL;
- return -1;
- }
+ if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
+ goto error;
+ err = archive_read_support_filter_xz(ctx->a);
+ if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
+ goto error;
+ if (archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK)
+ goto error;
+ if (archive_read_next_header(ctx->a, &e))
+ goto error;
+
return 0;
+
+error:
+ archive_read_free(ctx->a);
+ ctx->a = NULL;
+ return -1;
}
static ssize_t
--
David Marchand
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [EXT] Re: [PATCH 1/1] eal: enable xz read support and ignore warning
2023-09-25 9:10 ` David Marchand
@ 2023-09-26 13:32 ` Srikanth Yalavarthi
0 siblings, 0 replies; 10+ messages in thread
From: Srikanth Yalavarthi @ 2023-09-26 13:32 UTC (permalink / raw)
To: David Marchand
Cc: Aaron Conole, Igor Russkikh, dev,
Shivah Shankar Shankar Narayan Rao, Anup Prabhu, Prince Takkar,
jerinjacobk, stable, Srikanth Yalavarthi
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: 25 September 2023 14:40
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> <irusskikh@marvell.com>; dev@dpdk.org; Shivah Shankar Shankar Narayan
> Rao <sshankarnara@marvell.com>; Anup Prabhu <aprabhu@marvell.com>;
> Prince Takkar <ptakkar@marvell.com>; jerinjacobk@gmail.com;
> stable@dpdk.org; Srikanth Yalavarthi <syalavarthi@marvell.com>
> Subject: [EXT] Re: [PATCH 1/1] eal: enable xz read support and ignore
> warning
>
> External Email
>
> ----------------------------------------------------------------------
> Hello,
>
> Thank you for the patch.
>
> On Fri, Sep 22, 2023 at 6:54 PM Srikanth Yalavarthi
> <syalavarthi@marvell.com> wrote:
> >
> > archive_read_support_filter_xz returns a warning when compression is
> > not fully supported and is supported through external program. This
> > warning can be ignored when reading the files through firmware open as
> > only decompression is required.
>
> - I don't understand the last sentence, it seems to state something about
> *only* needing decompression support but well,
> archive_read_support_filter_xz (like archive_read_support_filter_* other
> helpers) *is about* decompressing a file.
What I meant is, in rte_firmware_read, we will be reading / decompressing the archive files.
Since support to write compressed files is required here, we can ignore the ARCHIVE_WARN.
>
>
> - I can't reproduce this ARCHIVE_WARN thing, not sure which libarchive you
> use, or which knob/build option triggered this behavior you observe.
> So I need you to to double check how this change affects the code.
>
In our build setup, we are cross-compiling libarchive without any dependency libs/headers like zlib, bzip2, lzma enabled.
I guess this is causing libarchive to be built without compression support.
> Please pass a xz-compressed mldev fw .bin file and confirm it still works.
I have tested 3 cases.
(1) ml-fw.bin -> A decompressed binary file
(2) ml-fw.bin.xz -> A compressed archive created from mlip-fw.bin (1)
(3) ml-fw.bin -> mlip-fw.bin.xz (2) renamed as ml-fw.bin
>
>
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > ---
> > lib/eal/unix/eal_firmware.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
> > index d1616b0bd9..05c06c222a 100644
> > --- a/lib/eal/unix/eal_firmware.c
> > +++ b/lib/eal/unix/eal_firmware.c
> > @@ -25,12 +25,19 @@ static int
> > firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t
> > blocksize) {
> > struct archive_entry *e;
> > + int err;
> >
> > ctx->a = archive_read_new();
> > if (ctx->a == NULL)
> > return -1;
> > +
> > + err = archive_read_support_filter_xz(ctx->a);
> > + if (err != ARCHIVE_OK && err != ARCHIVE_WARN) {
> > + ctx->a = NULL;
> > + return -1;
> > + }
>
> - This patch leaks ctx->a content on error.
>
> Plus I prefer we keep the original order of the code because it matches what
> libarchive does: first look for an archive format, then next look for
> compression matters.
>
> The simpler is to add an error label like I did in the debug patch.
> Something like:
Submitted v2 patch with suggested changes.
>
> diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c index
> d1616b0bd9..269688d550 100644
> --- a/lib/eal/unix/eal_firmware.c
> +++ b/lib/eal/unix/eal_firmware.c
> @@ -25,19 +25,27 @@ static int
> firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t
> blocksize)
> {
> struct archive_entry *e;
> + int err;
>
> ctx->a = archive_read_new();
> if (ctx->a == NULL)
> return -1;
> - if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
> - archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
> - archive_read_open_filename(ctx->a, name,
> blocksize) != ARCHIVE_OK ||
> - archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
> - archive_read_free(ctx->a);
> - ctx->a = NULL;
> - return -1;
> - }
> + if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
> + goto error;
> + err = archive_read_support_filter_xz(ctx->a);
> + if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
> + goto error;
> + if (archive_read_open_filename(ctx->a, name, blocksize) !=
> ARCHIVE_OK)
> + goto error;
> + if (archive_read_next_header(ctx->a, &e))
> + goto error;
> +
> return 0;
> +
> +error:
> + archive_read_free(ctx->a);
> + ctx->a = NULL;
> + return -1;
> }
>
> static ssize_t
>
>
> --
> David Marchand
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/1] eal: update xz read support and ignore warning
2023-09-22 16:53 [PATCH 1/1] eal: enable xz read support and ignore warning Srikanth Yalavarthi
2023-09-25 9:10 ` David Marchand
@ 2023-09-26 13:30 ` Srikanth Yalavarthi
2023-09-26 13:56 ` David Marchand
2023-09-26 13:49 ` [PATCH v3 " Srikanth Yalavarthi
2023-09-26 14:44 ` [PATCH v4 1/1] eal/unix: fix firmware reading with external xz helper Srikanth Yalavarthi
3 siblings, 1 reply; 10+ messages in thread
From: Srikanth Yalavarthi @ 2023-09-26 13:30 UTC (permalink / raw)
To: David Marchand, Aaron Conole, Igor Russkikh
Cc: dev, syalavarthi, sshankarnara, aprabhu, ptakkar, jerinjacobk, stable
archive_read_support_filter_xz returns a warning when
compression is not fully supported and is supported
through external program. This warning can be ignored
when reading the files through firmware open as only
decompression is required.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
Change-Id: I38cce556ec637af03dbde74d7d18318af48082d6
---
lib/eal/unix/eal_firmware.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
index d1616b0bd9..16690b4245 100644
--- a/lib/eal/unix/eal_firmware.c
+++ b/lib/eal/unix/eal_firmware.c
@@ -25,19 +25,31 @@ static int
firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t blocksize)
{
struct archive_entry *e;
+ int err;
ctx->a = archive_read_new();
if (ctx->a == NULL)
return -1;
- if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
- archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
- archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK ||
- archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
- archive_read_free(ctx->a);
- ctx->a = NULL;
- return -1;
- }
+
+ if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
+ goto error;
+
+ err = archive_read_support_filter_xz(ctx->a);
+ if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
+ goto error;
+
+ if (archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK)
+ goto error;
+
+ if (archive_read_next_header(ctx->a, &e))
+ goto error;
+
return 0;
+
+error:
+ archive_read_free(ctx->a);
+ ctx->a = NULL;
+ return -1;
}
static ssize_t
--
2.41.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/1] eal: update xz read support and ignore warning
2023-09-26 13:30 ` [PATCH v2 1/1] eal: update " Srikanth Yalavarthi
@ 2023-09-26 13:56 ` David Marchand
2023-09-26 14:47 ` [EXT] " Srikanth Yalavarthi
0 siblings, 1 reply; 10+ messages in thread
From: David Marchand @ 2023-09-26 13:56 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, dev, sshankarnara, aprabhu, ptakkar,
jerinjacobk, stable
On Tue, Sep 26, 2023 at 3:30 PM Srikanth Yalavarthi
<syalavarthi@marvell.com> wrote:
>
> archive_read_support_filter_xz returns a warning when
> compression is not fully supported and is supported
> through external program. This warning can be ignored
> when reading the files through firmware open as only
> decompression is required.
Same comment as before, this sentence is confusing.
Compressing files does not matter in DPDK rte_firmware_ API.
The commit title and commitlog won't help people understand in which
case the issue is reproduced, and how this patch fixes it.
Suggesting the following title and commitlog:
"""
eal/unix: fix firmware reading with external xz support
libarchive may support xz decompression only through an external
(slower) helper.
In such a case, archive_read_support_filter_xz() returns ARCHIVE_WARN.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
"""
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Change-Id: I38cce556ec637af03dbde74d7d18318af48082d6
This is internal scm stuff and must be dropped.
> ---
> lib/eal/unix/eal_firmware.c | 28 ++++++++++++++++++++--------
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
> index d1616b0bd9..16690b4245 100644
> --- a/lib/eal/unix/eal_firmware.c
> +++ b/lib/eal/unix/eal_firmware.c
> @@ -25,19 +25,31 @@ static int
> firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t blocksize)
> {
> struct archive_entry *e;
> + int err;
>
> ctx->a = archive_read_new();
> if (ctx->a == NULL)
> return -1;
> - if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
> - archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
> - archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK ||
> - archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
> - archive_read_free(ctx->a);
> - ctx->a = NULL;
> - return -1;
> - }
> +
> + if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
> + goto error;
> +
> + err = archive_read_support_filter_xz(ctx->a);
> + if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
> + goto error;
> +
> + if (archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK)
> + goto error;
> +
> + if (archive_read_next_header(ctx->a, &e))
I did a typo when quickly writting the snippet previously.
It should be "archive_read_next_header(ctx->a, &e) != ARCHIVE_OK"
> + goto error;
> +
> return 0;
> +
> +error:
> + archive_read_free(ctx->a);
> + ctx->a = NULL;
> + return -1;
> }
>
> static ssize_t
> --
> 2.41.0
>
--
David Marchand
^ permalink raw reply [flat|nested] 10+ messages in thread* RE: [EXT] Re: [PATCH v2 1/1] eal: update xz read support and ignore warning
2023-09-26 13:56 ` David Marchand
@ 2023-09-26 14:47 ` Srikanth Yalavarthi
0 siblings, 0 replies; 10+ messages in thread
From: Srikanth Yalavarthi @ 2023-09-26 14:47 UTC (permalink / raw)
To: David Marchand
Cc: Aaron Conole, Igor Russkikh, dev,
Shivah Shankar Shankar Narayan Rao, Anup Prabhu, Prince Takkar,
jerinjacobk, stable, Srikanth Yalavarthi
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: 26 September 2023 19:27
> To: Srikanth Yalavarthi <syalavarthi@marvell.com>
> Cc: Aaron Conole <aconole@redhat.com>; Igor Russkikh
> <irusskikh@marvell.com>; dev@dpdk.org; Shivah Shankar Shankar Narayan
> Rao <sshankarnara@marvell.com>; Anup Prabhu <aprabhu@marvell.com>;
> Prince Takkar <ptakkar@marvell.com>; jerinjacobk@gmail.com;
> stable@dpdk.org
> Subject: [EXT] Re: [PATCH v2 1/1] eal: update xz read support and ignore
> warning
>
> External Email
>
> ----------------------------------------------------------------------
> On Tue, Sep 26, 2023 at 3:30 PM Srikanth Yalavarthi
> <syalavarthi@marvell.com> wrote:
> >
> > archive_read_support_filter_xz returns a warning when compression is
> > not fully supported and is supported through external program. This
> > warning can be ignored when reading the files through firmware open as
> > only decompression is required.
>
> Same comment as before, this sentence is confusing.
> Compressing files does not matter in DPDK rte_firmware_ API.
>
> The commit title and commitlog won't help people understand in which case
> the issue is reproduced, and how this patch fixes it.
>
> Suggesting the following title and commitlog:
>
> """
> eal/unix: fix firmware reading with external xz support
>
> libarchive may support xz decompression only through an external
> (slower) helper.
> In such a case, archive_read_support_filter_xz() returns ARCHIVE_WARN.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
> """
>
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
> > Change-Id: I38cce556ec637af03dbde74d7d18318af48082d6
>
> This is internal scm stuff and must be dropped.
>
> > ---
> > lib/eal/unix/eal_firmware.c | 28 ++++++++++++++++++++--------
> > 1 file changed, 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
> > index d1616b0bd9..16690b4245 100644
> > --- a/lib/eal/unix/eal_firmware.c
> > +++ b/lib/eal/unix/eal_firmware.c
> > @@ -25,19 +25,31 @@ static int
> > firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t
> > blocksize) {
> > struct archive_entry *e;
> > + int err;
> >
> > ctx->a = archive_read_new();
> > if (ctx->a == NULL)
> > return -1;
> > - if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
> > - archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
> > - archive_read_open_filename(ctx->a, name, blocksize) !=
> ARCHIVE_OK ||
> > - archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
> > - archive_read_free(ctx->a);
> > - ctx->a = NULL;
> > - return -1;
> > - }
> > +
> > + if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
> > + goto error;
> > +
> > + err = archive_read_support_filter_xz(ctx->a);
> > + if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
> > + goto error;
> > +
> > + if (archive_read_open_filename(ctx->a, name, blocksize) !=
> ARCHIVE_OK)
> > + goto error;
> > +
> > + if (archive_read_next_header(ctx->a, &e))
>
> I did a typo when quickly writting the snippet previously.
> It should be "archive_read_next_header(ctx->a, &e) != ARCHIVE_OK"
>
>
> > + goto error;
> > +
> > return 0;
> > +
> > +error:
> > + archive_read_free(ctx->a);
> > + ctx->a = NULL;
> > + return -1;
> > }
> >
> > static ssize_t
> > --
> > 2.41.0
> >
>
Submitted patch version 4 with suggested changes.
>
> --
> David Marchand
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/1] eal: update xz read support and ignore warning
2023-09-22 16:53 [PATCH 1/1] eal: enable xz read support and ignore warning Srikanth Yalavarthi
2023-09-25 9:10 ` David Marchand
2023-09-26 13:30 ` [PATCH v2 1/1] eal: update " Srikanth Yalavarthi
@ 2023-09-26 13:49 ` Srikanth Yalavarthi
2023-09-26 14:44 ` [PATCH v4 1/1] eal/unix: fix firmware reading with external xz helper Srikanth Yalavarthi
3 siblings, 0 replies; 10+ messages in thread
From: Srikanth Yalavarthi @ 2023-09-26 13:49 UTC (permalink / raw)
To: Aaron Conole, Igor Russkikh, David Marchand
Cc: dev, syalavarthi, sshankarnara, aprabhu, ptakkar, jerinjacobk, stable
archive_read_support_filter_xz returns a warning when
compression is not fully supported and is supported
through external program. This warning can be ignored
when reading the files through firmware open as only
decompression is required.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v3:
* removed gerrit change-id
v2:
* updated code as per review comments
lib/eal/unix/eal_firmware.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
index d1616b0bd9..16690b4245 100644
--- a/lib/eal/unix/eal_firmware.c
+++ b/lib/eal/unix/eal_firmware.c
@@ -25,19 +25,31 @@ static int
firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t blocksize)
{
struct archive_entry *e;
+ int err;
ctx->a = archive_read_new();
if (ctx->a == NULL)
return -1;
- if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
- archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
- archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK ||
- archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
- archive_read_free(ctx->a);
- ctx->a = NULL;
- return -1;
- }
+
+ if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
+ goto error;
+
+ err = archive_read_support_filter_xz(ctx->a);
+ if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
+ goto error;
+
+ if (archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK)
+ goto error;
+
+ if (archive_read_next_header(ctx->a, &e))
+ goto error;
+
return 0;
+
+error:
+ archive_read_free(ctx->a);
+ ctx->a = NULL;
+ return -1;
}
static ssize_t
--
2.41.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 1/1] eal/unix: fix firmware reading with external xz helper
2023-09-22 16:53 [PATCH 1/1] eal: enable xz read support and ignore warning Srikanth Yalavarthi
` (2 preceding siblings ...)
2023-09-26 13:49 ` [PATCH v3 " Srikanth Yalavarthi
@ 2023-09-26 14:44 ` Srikanth Yalavarthi
2023-09-27 8:20 ` David Marchand
3 siblings, 1 reply; 10+ messages in thread
From: Srikanth Yalavarthi @ 2023-09-26 14:44 UTC (permalink / raw)
To: Aaron Conole, Igor Russkikh, David Marchand
Cc: dev, syalavarthi, sshankarnara, aprabhu, ptakkar, jerinjacobk, stable
libarchive may support xz decompression only through an
external (slower) helper.
In such a case, archive_read_support_filter_xz() returns
ARCHIVE_WARN.
Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
Cc: stable@dpdk.org
Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
---
v4:
* updated commit message
* fixed checking archive_read_next_header
v3:
* removed gerrit change-id
v2:
* updated code as per review comments
lib/eal/unix/eal_firmware.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
index d1616b0bd9..1a7cf8e7b7 100644
--- a/lib/eal/unix/eal_firmware.c
+++ b/lib/eal/unix/eal_firmware.c
@@ -25,19 +25,31 @@ static int
firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t blocksize)
{
struct archive_entry *e;
+ int err;
ctx->a = archive_read_new();
if (ctx->a == NULL)
return -1;
- if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
- archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
- archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK ||
- archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
- archive_read_free(ctx->a);
- ctx->a = NULL;
- return -1;
- }
+
+ if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK)
+ goto error;
+
+ err = archive_read_support_filter_xz(ctx->a);
+ if (err != ARCHIVE_OK && err != ARCHIVE_WARN)
+ goto error;
+
+ if (archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK)
+ goto error;
+
+ if (archive_read_next_header(ctx->a, &e) != ARCHIVE_OK)
+ goto error;
+
return 0;
+
+error:
+ archive_read_free(ctx->a);
+ ctx->a = NULL;
+ return -1;
}
static ssize_t
--
2.41.0
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 1/1] eal/unix: fix firmware reading with external xz helper
2023-09-26 14:44 ` [PATCH v4 1/1] eal/unix: fix firmware reading with external xz helper Srikanth Yalavarthi
@ 2023-09-27 8:20 ` David Marchand
2023-09-27 9:35 ` David Marchand
0 siblings, 1 reply; 10+ messages in thread
From: David Marchand @ 2023-09-27 8:20 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, dev, sshankarnara, aprabhu, ptakkar,
jerinjacobk, stable
On Tue, Sep 26, 2023 at 4:45 PM Srikanth Yalavarthi
<syalavarthi@marvell.com> wrote:
>
> libarchive may support xz decompression only through an
> external (slower) helper.
>
> In such a case, archive_read_support_filter_xz() returns
> ARCHIVE_WARN.
>
> Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> Cc: stable@dpdk.org
>
> Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
--
David Marchand
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v4 1/1] eal/unix: fix firmware reading with external xz helper
2023-09-27 8:20 ` David Marchand
@ 2023-09-27 9:35 ` David Marchand
0 siblings, 0 replies; 10+ messages in thread
From: David Marchand @ 2023-09-27 9:35 UTC (permalink / raw)
To: Srikanth Yalavarthi
Cc: Aaron Conole, Igor Russkikh, dev, sshankarnara, aprabhu, ptakkar,
jerinjacobk, stable
On Wed, Sep 27, 2023 at 10:20 AM David Marchand
<david.marchand@redhat.com> wrote:
> On Tue, Sep 26, 2023 at 4:45 PM Srikanth Yalavarthi
> <syalavarthi@marvell.com> wrote:
> >
> > libarchive may support xz decompression only through an
> > external (slower) helper.
> >
> > In such a case, archive_read_support_filter_xz() returns
> > ARCHIVE_WARN.
> >
> > Fixes: 40edb9c0d36b ("eal: handle compressed firmware")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Srikanth Yalavarthi <syalavarthi@marvell.com>
>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
Applied, thanks.
--
David Marchand
^ permalink raw reply [flat|nested] 10+ messages in thread