[PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Peter Wu]

/proc/config has never existed as far as I can see, but /proc/config.gz
is present on Arch Linux. Add support for decompressing config.gz using
zlib which is a mandatory dependency of libelf. Replace existing stdio
functions with gzFile operations since the latter transparently handles
uncompressed and gzip-compressed files.

Cc: Quentin Monnet <quentin.monnet@netronome.com>
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
 v3: replace popen(gunzip) by linking directly to zlib. Reword commit
     message, remove "Fixes" line. (this patch)
 v2: fix style (reorder vars as reverse xmas tree, rename function,
     braces), fallback to /proc/config.gz if uname() fails.
     https://lkml.kernel.org/r/20190806010702.3303-1-peter@lekensteyn.nl
 v1: https://lkml.kernel.org/r/20190805001541.8096-1-peter@lekensteyn.nl

Hi,

Thanks to Jakub for observing that zlib is already used by libelf, this
simplifies the patch tremendously as the same API can be used for both
compressed and uncompressed files. No special case exists anymore for
fclose/pclose.

According to configure.ac in elfutils, zlib is mandatory, so I just
assume it to be available. For simplicity I also silently assume lines
to be less than 4096 characters. If that is not the case, then lines
will appear truncated, but that should not be an issue for the
CONFIG_xyz lines that we are scanning for.

Jakub requested the handle leak fix to be posted separately against the
bpf tree, but since the whole code is rewritten I am not sure if it is
worth it. It is an unusual edge case: /boot/config-$(uname -r) could be
opened, but starts with unexpected data.

Kind regards,
Peter
---
 tools/bpf/bpftool/Makefile  |   2 +-
 tools/bpf/bpftool/feature.c | 105 ++++++++++++++++++------------------
 2 files changed, 54 insertions(+), 53 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index a7afea4dec47..078bd0dcfba5 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -52,7 +52,7 @@ ifneq ($(EXTRA_LDFLAGS),)
 LDFLAGS += $(EXTRA_LDFLAGS)
 endif
 
-LIBS = -lelf $(LIBBPF)
+LIBS = -lelf -lz $(LIBBPF)
 
 INSTALL ?= install
 RM ?= rm -f
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index d672d9086fff..03bdc5b3ac49 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -14,6 +14,7 @@
 
 #include <bpf.h>
 #include <libbpf.h>
+#include <zlib.h>
 
 #include "main.h"
 
@@ -284,34 +285,32 @@ static void probe_jit_limit(void)
 	}
 }
 
-static char *get_kernel_config_option(FILE *fd, const char *option)
+static bool read_next_kernel_config_option(gzFile file, char *buf, size_t n,
+					   char **value)
 {
-	size_t line_n = 0, optlen = strlen(option);
-	char *res, *strval, *line = NULL;
-	ssize_t n;
+	char *sep;
 
-	rewind(fd);
-	while ((n = getline(&line, &line_n, fd)) > 0) {
-		if (strncmp(line, option, optlen))
+	while (gzgets(file, buf, n)) {
+		if (strncmp(buf, "CONFIG_", 7))
 			continue;
-		/* Check we have at least '=', value, and '\n' */
-		if (strlen(line) < optlen + 3)
-			continue;
-		if (*(line + optlen) != '=')
+
+		sep = strchr(buf, '=');
+		if (!sep)
 			continue;
 
 		/* Trim ending '\n' */
-		line[strlen(line) - 1] = '\0';
+		buf[strlen(buf) - 1] = '\0';
+
+		/* Split on '=' and ensure that a value is present. */
+		*sep = '\0';
+		if (!sep[1])
+			continue;
 
-		/* Copy and return config option value */
-		strval = line + optlen + 1;
-		res = strdup(strval);
-		free(line);
-		return res;
+		*value = sep + 1;
+		return true;
 	}
-	free(line);
 
-	return NULL;
+	return false;
 }
 
 static void probe_kernel_image_config(void)
@@ -386,59 +385,61 @@ static void probe_kernel_image_config(void)
 		/* test_bpf module for BPF tests */
 		"CONFIG_TEST_BPF",
 	};
-	char *value, *buf = NULL;
+	char *values[ARRAY_SIZE(options)] = { };
 	struct utsname utsn;
 	char path[PATH_MAX];
-	size_t i, n;
-	ssize_t ret;
-	FILE *fd;
+	gzFile file = NULL;
+	char buf[4096];
+	char *value;
+	size_t i;
 
-	if (uname(&utsn))
-		goto no_config;
+	if (!uname(&utsn)) {
+		snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
 
-	snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
+		/* gzopen also accepts uncompressed files. */
+		file = gzopen(path, "r");
+	}
 
-	fd = fopen(path, "r");
-	if (!fd && errno == ENOENT) {
-		/* Some distributions put the config file at /proc/config, give
-		 * it a try.
-		 * Sometimes it is also at /proc/config.gz but we do not try
-		 * this one for now, it would require linking against libz.
+	if (!file) {
+		/* Some distributions build with CONFIG_IKCONFIG=y and put the
+		 * config file at /proc/config.gz.
 		 */
-		fd = fopen("/proc/config", "r");
+		file = gzopen("/proc/config.gz", "r");
 	}
-	if (!fd) {
+	if (!file) {
 		p_info("skipping kernel config, can't open file: %s",
 		       strerror(errno));
-		goto no_config;
+		goto end_parse;
 	}
 	/* Sanity checks */
-	ret = getline(&buf, &n, fd);
-	ret = getline(&buf, &n, fd);
-	if (!buf || !ret) {
+	if (!gzgets(file, buf, sizeof(buf)) ||
+	    !gzgets(file, buf, sizeof(buf))) {
 		p_info("skipping kernel config, can't read from file: %s",
 		       strerror(errno));
-		free(buf);
-		goto no_config;
+		goto end_parse;
 	}
 	if (strcmp(buf, "# Automatically generated file; DO NOT EDIT.\n")) {
 		p_info("skipping kernel config, can't find correct file");
-		free(buf);
-		goto no_config;
+		goto end_parse;
 	}
-	free(buf);
 
-	for (i = 0; i < ARRAY_SIZE(options); i++) {
-		value = get_kernel_config_option(fd, options[i]);
-		print_kernel_option(options[i], value);
-		free(value);
+	while (read_next_kernel_config_option(file, buf, sizeof(buf), &value)) {
+		for (i = 0; i < ARRAY_SIZE(options); i++) {
+			if (values[i] || strcmp(buf, options[i]))
+				continue;
+
+			values[i] = strdup(value);
+		}
 	}
-	fclose(fd);
-	return;
 
-no_config:
-	for (i = 0; i < ARRAY_SIZE(options); i++)
-		print_kernel_option(options[i], NULL);
+end_parse:
+	if (file)
+		gzclose(file);
+
+	for (i = 0; i < ARRAY_SIZE(options); i++) {
+		print_kernel_option(options[i], values[i]);
+		free(values[i]);
+	}
 }
 
 static bool probe_bpf_syscall(const char *define_prefix)
-- 
2.22.0

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Stanislav Fomichev]

On 08/09, Peter Wu wrote:
> /proc/config has never existed as far as I can see, but /proc/config.gz
> is present on Arch Linux. Add support for decompressing config.gz using
> zlib which is a mandatory dependency of libelf. Replace existing stdio
> functions with gzFile operations since the latter transparently handles
> uncompressed and gzip-compressed files.
> 
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>  v3: replace popen(gunzip) by linking directly to zlib. Reword commit
>      message, remove "Fixes" line. (this patch)
>  v2: fix style (reorder vars as reverse xmas tree, rename function,
>      braces), fallback to /proc/config.gz if uname() fails.
>      https://lkml.kernel.org/r/20190806010702.3303-1-peter@lekensteyn.nl
>  v1: https://lkml.kernel.org/r/20190805001541.8096-1-peter@lekensteyn.nl
> 
> Hi,
> 
> Thanks to Jakub for observing that zlib is already used by libelf, this
> simplifies the patch tremendously as the same API can be used for both
> compressed and uncompressed files. No special case exists anymore for
> fclose/pclose.
> 
> According to configure.ac in elfutils, zlib is mandatory, so I just
> assume it to be available. For simplicity I also silently assume lines
> to be less than 4096 characters. If that is not the case, then lines
> will appear truncated, but that should not be an issue for the
> CONFIG_xyz lines that we are scanning for.
> 
> Jakub requested the handle leak fix to be posted separately against the
> bpf tree, but since the whole code is rewritten I am not sure if it is
> worth it. It is an unusual edge case: /boot/config-$(uname -r) could be
> opened, but starts with unexpected data.
> 
> Kind regards,
> Peter
> ---
>  tools/bpf/bpftool/Makefile  |   2 +-
>  tools/bpf/bpftool/feature.c | 105 ++++++++++++++++++------------------
>  2 files changed, 54 insertions(+), 53 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index a7afea4dec47..078bd0dcfba5 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -52,7 +52,7 @@ ifneq ($(EXTRA_LDFLAGS),)
>  LDFLAGS += $(EXTRA_LDFLAGS)
>  endif
>  
> -LIBS = -lelf $(LIBBPF)
> +LIBS = -lelf -lz $(LIBBPF)
You're saying in the commit description that bpftool already links
against -lz (via -lelf), but then explicitly add -lz here, why?

>  INSTALL ?= install
>  RM ?= rm -f
> diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
> index d672d9086fff..03bdc5b3ac49 100644
> --- a/tools/bpf/bpftool/feature.c
> +++ b/tools/bpf/bpftool/feature.c
> @@ -14,6 +14,7 @@
>  
>  #include <bpf.h>
>  #include <libbpf.h>
> +#include <zlib.h>
>  
>  #include "main.h"
>  
> @@ -284,34 +285,32 @@ static void probe_jit_limit(void)
>  	}
>  }
>  
> -static char *get_kernel_config_option(FILE *fd, const char *option)
> +static bool read_next_kernel_config_option(gzFile file, char *buf, size_t n,
> +					   char **value)
>  {
> -	size_t line_n = 0, optlen = strlen(option);
> -	char *res, *strval, *line = NULL;
> -	ssize_t n;
> +	char *sep;
>  
> -	rewind(fd);
> -	while ((n = getline(&line, &line_n, fd)) > 0) {
> -		if (strncmp(line, option, optlen))
> +	while (gzgets(file, buf, n)) {
> +		if (strncmp(buf, "CONFIG_", 7))
>  			continue;
> -		/* Check we have at least '=', value, and '\n' */
> -		if (strlen(line) < optlen + 3)
> -			continue;
> -		if (*(line + optlen) != '=')
> +
> +		sep = strchr(buf, '=');
> +		if (!sep)
>  			continue;
>  
>  		/* Trim ending '\n' */
> -		line[strlen(line) - 1] = '\0';
> +		buf[strlen(buf) - 1] = '\0';
> +
> +		/* Split on '=' and ensure that a value is present. */
> +		*sep = '\0';
> +		if (!sep[1])
> +			continue;
>  
> -		/* Copy and return config option value */
> -		strval = line + optlen + 1;
> -		res = strdup(strval);
> -		free(line);
> -		return res;
> +		*value = sep + 1;
> +		return true;
>  	}
> -	free(line);
>  
> -	return NULL;
> +	return false;
>  }
>  
>  static void probe_kernel_image_config(void)
> @@ -386,59 +385,61 @@ static void probe_kernel_image_config(void)
>  		/* test_bpf module for BPF tests */
>  		"CONFIG_TEST_BPF",
>  	};
> -	char *value, *buf = NULL;
> +	char *values[ARRAY_SIZE(options)] = { };
>  	struct utsname utsn;
>  	char path[PATH_MAX];
> -	size_t i, n;
> -	ssize_t ret;
> -	FILE *fd;
> +	gzFile file = NULL;
> +	char buf[4096];
> +	char *value;
> +	size_t i;
>  
> -	if (uname(&utsn))
> -		goto no_config;
> +	if (!uname(&utsn)) {
> +		snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
>  
> -	snprintf(path, sizeof(path), "/boot/config-%s", utsn.release);
> +		/* gzopen also accepts uncompressed files. */
> +		file = gzopen(path, "r");
> +	}
>  
> -	fd = fopen(path, "r");
> -	if (!fd && errno == ENOENT) {
> -		/* Some distributions put the config file at /proc/config, give
> -		 * it a try.
> -		 * Sometimes it is also at /proc/config.gz but we do not try
> -		 * this one for now, it would require linking against libz.
> +	if (!file) {
> +		/* Some distributions build with CONFIG_IKCONFIG=y and put the
> +		 * config file at /proc/config.gz.
>  		 */
> -		fd = fopen("/proc/config", "r");
> +		file = gzopen("/proc/config.gz", "r");
>  	}
> -	if (!fd) {
> +	if (!file) {
>  		p_info("skipping kernel config, can't open file: %s",
>  		       strerror(errno));
> -		goto no_config;
> +		goto end_parse;
>  	}
>  	/* Sanity checks */
> -	ret = getline(&buf, &n, fd);
> -	ret = getline(&buf, &n, fd);
> -	if (!buf || !ret) {
> +	if (!gzgets(file, buf, sizeof(buf)) ||
> +	    !gzgets(file, buf, sizeof(buf))) {
>  		p_info("skipping kernel config, can't read from file: %s",
>  		       strerror(errno));
> -		free(buf);
> -		goto no_config;
> +		goto end_parse;
>  	}
>  	if (strcmp(buf, "# Automatically generated file; DO NOT EDIT.\n")) {
>  		p_info("skipping kernel config, can't find correct file");
> -		free(buf);
> -		goto no_config;
> +		goto end_parse;
>  	}
> -	free(buf);
>  
> -	for (i = 0; i < ARRAY_SIZE(options); i++) {
> -		value = get_kernel_config_option(fd, options[i]);
> -		print_kernel_option(options[i], value);
> -		free(value);
> +	while (read_next_kernel_config_option(file, buf, sizeof(buf), &value)) {
> +		for (i = 0; i < ARRAY_SIZE(options); i++) {
> +			if (values[i] || strcmp(buf, options[i]))
> +				continue;
> +
> +			values[i] = strdup(value);
> +		}
>  	}
> -	fclose(fd);
> -	return;
>  
> -no_config:
> -	for (i = 0; i < ARRAY_SIZE(options); i++)
> -		print_kernel_option(options[i], NULL);
> +end_parse:
> +	if (file)
> +		gzclose(file);
> +
> +	for (i = 0; i < ARRAY_SIZE(options); i++) {
> +		print_kernel_option(options[i], values[i]);
> +		free(values[i]);
> +	}
>  }
>  
>  static bool probe_bpf_syscall(const char *define_prefix)
> -- 
> 2.22.0
> 

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Jakub Kicinski]

On Fri, 9 Aug 2019 08:32:10 -0700, Stanislav Fomichev wrote:
> On 08/09, Peter Wu wrote:
> > /proc/config has never existed as far as I can see, but /proc/config.gz
> > is present on Arch Linux. Add support for decompressing config.gz using
> > zlib which is a mandatory dependency of libelf. Replace existing stdio
> > functions with gzFile operations since the latter transparently handles
> > uncompressed and gzip-compressed files.
> > 
> > Cc: Quentin Monnet <quentin.monnet@netronome.com>
> > Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Thanks for the patch, looks good to me now!

> >  tools/bpf/bpftool/Makefile  |   2 +-
> >  tools/bpf/bpftool/feature.c | 105 ++++++++++++++++++------------------
> >  2 files changed, 54 insertions(+), 53 deletions(-)
> > 
> > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > index a7afea4dec47..078bd0dcfba5 100644
> > --- a/tools/bpf/bpftool/Makefile
> > +++ b/tools/bpf/bpftool/Makefile
> > @@ -52,7 +52,7 @@ ifneq ($(EXTRA_LDFLAGS),)
> >  LDFLAGS += $(EXTRA_LDFLAGS)
> >  endif
> >  
> > -LIBS = -lelf $(LIBBPF)
> > +LIBS = -lelf -lz $(LIBBPF)  
> You're saying in the commit description that bpftool already links
> against -lz (via -lelf), but then explicitly add -lz here, why?

It probably won't hurt to enable the zlib test:

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 078bd0dcfba5..8176632e519c 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -58,8 +58,8 @@ INSTALL ?= install
 RM ?= rm -f
 
 FEATURE_USER = .bpftool
-FEATURE_TESTS = libbfd disassembler-four-args reallocarray
-FEATURE_DISPLAY = libbfd disassembler-four-args
+FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
+FEATURE_DISPLAY = libbfd disassembler-four-args zlib
 
 check_feat := 1
 NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall

And then we can test for it the way libbpf tests for elf:

all: zdep $(OUTPUT)bpftool

PHONY += zdep

zdep:
	@if [ "$(feature-zlib)" != "1" ]; then echo "No zlib found"; exit 1 ; fi

Or maybe just $(error ...), Stan what's your preference here? 
We don't have a precedent for hard tests of features in bpftool.

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Stanislav Fomichev]

On 08/09, Jakub Kicinski wrote:
> On Fri, 9 Aug 2019 08:32:10 -0700, Stanislav Fomichev wrote:
> > On 08/09, Peter Wu wrote:
> > > /proc/config has never existed as far as I can see, but /proc/config.gz
> > > is present on Arch Linux. Add support for decompressing config.gz using
> > > zlib which is a mandatory dependency of libelf. Replace existing stdio
> > > functions with gzFile operations since the latter transparently handles
> > > uncompressed and gzip-compressed files.
> > > 
> > > Cc: Quentin Monnet <quentin.monnet@netronome.com>
> > > Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> 
> Thanks for the patch, looks good to me now!
> 
> > >  tools/bpf/bpftool/Makefile  |   2 +-
> > >  tools/bpf/bpftool/feature.c | 105 ++++++++++++++++++------------------
> > >  2 files changed, 54 insertions(+), 53 deletions(-)
> > > 
> > > diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> > > index a7afea4dec47..078bd0dcfba5 100644
> > > --- a/tools/bpf/bpftool/Makefile
> > > +++ b/tools/bpf/bpftool/Makefile
> > > @@ -52,7 +52,7 @@ ifneq ($(EXTRA_LDFLAGS),)
> > >  LDFLAGS += $(EXTRA_LDFLAGS)
> > >  endif
> > >  
> > > -LIBS = -lelf $(LIBBPF)
> > > +LIBS = -lelf -lz $(LIBBPF)  
> > You're saying in the commit description that bpftool already links
> > against -lz (via -lelf), but then explicitly add -lz here, why?
> 
> It probably won't hurt to enable the zlib test:
> 
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 078bd0dcfba5..8176632e519c 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -58,8 +58,8 @@ INSTALL ?= install
>  RM ?= rm -f
>  
>  FEATURE_USER = .bpftool
> -FEATURE_TESTS = libbfd disassembler-four-args reallocarray
> -FEATURE_DISPLAY = libbfd disassembler-four-args
> +FEATURE_TESTS = libbfd disassembler-four-args reallocarray zlib
> +FEATURE_DISPLAY = libbfd disassembler-four-args zlib
>  
>  check_feat := 1
>  NON_CHECK_FEAT_TARGETS := clean uninstall doc doc-clean doc-install doc-uninstall
> 
> And then we can test for it the way libbpf tests for elf:
> 
> all: zdep $(OUTPUT)bpftool
> 
> PHONY += zdep
> 
> zdep:
> 	@if [ "$(feature-zlib)" != "1" ]; then echo "No zlib found"; exit 1 ; fi
> 
> Or maybe just $(error ...), Stan what's your preference here? 
> We don't have a precedent for hard tests of features in bpftool.
I'm just being nit picky :-)
Because changelog says we already depend on -lz, but then in the patch
we explicitly add it.

I think you were right in pointing out that we already implicitly depend
on -lz via -lelf and/or -lbfd. And it works for non-static builds.
We don't need an explicit -lz unless somebody puts '-static' in
EXTRA_CFLAGS. So maybe we should just submit the patch as is because
it fixes make EXTRA_CFLAGS=-static.

RE $(error): we don't do it for -lelf, right? So probably not worth
the hassle for zlib.

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Jakub Kicinski]

On Fri, 9 Aug 2019 14:48:31 -0700, Stanislav Fomichev wrote:
> I'm just being nit picky :-)
> Because changelog says we already depend on -lz, but then in the patch
> we explicitly add it.
> 
> I think you were right in pointing out that we already implicitly depend
> on -lz via -lelf and/or -lbfd. And it works for non-static builds.
> We don't need an explicit -lz unless somebody puts '-static' in
> EXTRA_CFLAGS. So maybe we should just submit the patch as is because
> it fixes make EXTRA_CFLAGS=-static.

Mm. Sounds reasonable. Fixing EXTRA_CFLAGS=-static would be really cool,
too, I always struggle to get a statically linked build.

> RE $(error): we don't do it for -lelf, right? So probably not worth
> the hassle for zlib.

Right, OTOH bpftool doesn't really care about -lelf, it's libbpf that
needs it, and libbpf does test.

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Peter Wu]

Hi all,

Thanks for the lovely feedback :)

On Fri, Aug 09, 2019 at 02:57:26PM -0700, Jakub Kicinski wrote:
> On Fri, 9 Aug 2019 14:48:31 -0700, Stanislav Fomichev wrote:
> > I'm just being nit picky :-)
> > Because changelog says we already depend on -lz, but then in the patch
> > we explicitly add it.

What I meant by that is that zlib is not a new dependency since it is
already a mandatory dependency of libelf which is currently marked as
mandatory dependency in bpftool. That is why I did not bother with
adding a feature test either since it would be redundant.

Adding an explicit dependency helps if you want to build bpftool as
static binary, or if libelf somehow drops zlib in the future.
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Quentin Monnet]

2019-08-09 01:39 UTC+0100 ~ Peter Wu <peter@lekensteyn.nl>
> /proc/config has never existed as far as I can see, but /proc/config.gz
> is present on Arch Linux. Add support for decompressing config.gz using
> zlib which is a mandatory dependency of libelf. Replace existing stdio
> functions with gzFile operations since the latter transparently handles
> uncompressed and gzip-compressed files.
> 
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>  v3: replace popen(gunzip) by linking directly to zlib. Reword commit
>      message, remove "Fixes" line. (this patch)
>  v2: fix style (reorder vars as reverse xmas tree, rename function,
>      braces), fallback to /proc/config.gz if uname() fails.
>      https://lkml.kernel.org/r/20190806010702.3303-1-peter@lekensteyn.nl
>  v1: https://lkml.kernel.org/r/20190805001541.8096-1-peter@lekensteyn.nl
> 
> Hi,
> 
> Thanks to Jakub for observing that zlib is already used by libelf, this
> simplifies the patch tremendously as the same API can be used for both
> compressed and uncompressed files. No special case exists anymore for
> fclose/pclose.
> 
> According to configure.ac in elfutils, zlib is mandatory, so I just
> assume it to be available. For simplicity I also silently assume lines
> to be less than 4096 characters. If that is not the case, then lines
> will appear truncated, but that should not be an issue for the
> CONFIG_xyz lines that we are scanning for.
> 
> Jakub requested the handle leak fix to be posted separately against the
> bpf tree, but since the whole code is rewritten I am not sure if it is
> worth it. It is an unusual edge case: /boot/config-$(uname -r) could be
> opened, but starts with unexpected data.
> 
> Kind regards,
> Peter

This version seems good to me, thanks!

Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>

Re: [PATCH v3] tools: bpftool: fix reading from /proc/config.gz

[Daniel Borkmann]

On 8/9/19 2:39 AM, Peter Wu wrote:
> /proc/config has never existed as far as I can see, but /proc/config.gz
> is present on Arch Linux. Add support for decompressing config.gz using
> zlib which is a mandatory dependency of libelf. Replace existing stdio
> functions with gzFile operations since the latter transparently handles
> uncompressed and gzip-compressed files.
> 
> Cc: Quentin Monnet <quentin.monnet@netronome.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

Applied, thanks. Please follow-up with a zlib feature test as suggested
by Jakub.