Re: [dpdk-dev] [PATCH 2/2] eal: handle compressed firmwares

[David Marchand <david.marchand@redhat.com>]

On Wed, Jun 2, 2021 at 1:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > +static int
> > +firmware_read(const char *name, void **buf, size_t *bufsz)
> > +{
> > +       const size_t blocksize = 4096;
> > +       int ret = -1;
> > +       int err;
> > +#ifdef RTE_HAS_LIBARCHIVE
>
>
> I think, better to have small inline functions for libarchive variant
> vs normal file accessors
> in the group for open, access, read etc to avoid the ifdef clutter and
> manage with one ifdef.

That may be a bit artificial, since there is no reuse of such helpers for now.
I'll have a try and see how it looks.


> > +int
> > +rte_firmware_read(const char *name, void **buf, size_t *bufsz)
> > +{
> > +       char path[PATH_MAX];
> > +       int ret;
> > +
> > +       ret = firmware_read(name, buf, bufsz);
> > +       if (ret < 0) {
> > +               snprintf(path, sizeof(path), "%s.xz", name);
> > +               path[PATH_MAX - 1] = '\0';
> > +#ifndef RTE_HAS_LIBARCHIVE
>
> See above

There is nothing to abstract here.

If you don't have libarchive, returning the .xz content to a driver is wrong.
I prefer to leave this block as is.



>
> > +               if (access(path, F_OK) == 0) {
> > +                       RTE_LOG(WARNING, EAL, "libarchive not available, %s cannot be decompressed\n",
> > +                               path);
> > +               }
> > +#else
> > +               ret = firmware_read(path, buf, bufsz);
> > +#endif
> >
>

-- 
David Marchand