On Wed, Jun 2, 2021 at 1:13 PM Jerin Jacob [off-list ref] wrote:

quoted

+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.

quoted

+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.

quoted

+               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