On Wed, Jun 2, 2021 at 1:13 PM Jerin Jacob <jerinjac...@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
