[PATCH] srcfiles.cxx: Prevent fd and entry leak
Make sure to free fd and the archive entry if an error is encountered
while adding source files to the archive.
Signed-off-by: Aaron Merey
---
src/srcfiles.cxx | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/srcfiles.cxx b/src/srcfiles.cxx
index c466b307..b6dd75ad 100644
--- a/src/srcfiles.cxx
+++ b/src/srcfiles.cxx
@@ -312,7 +312,6 @@ void zip_files()
struct stat st;
char buff[BUFFER_SIZE];
int len;
- int fd;
#ifdef ENABLE_LIBDEBUGINFOD
/* Initialize a debuginfod client. */
static unique_ptr
@@ -325,8 +324,10 @@ void zip_files()
int missing_files = 0;
for (const auto &pair : debug_sourcefiles)
{
-fd = -1;
+int fd = -1;
const std::string &file_path = pair.first;
+struct archive_entry *entry = NULL;
+string entry_name;
/* Attempt to query debuginfod client to fetch source files. */
#ifdef ENABLE_LIBDEBUGINFOD
@@ -353,8 +354,8 @@ void zip_files()
if (!no_backup)
#endif /* ENABLE_LIBDEBUGINFOD */
/* Files could not be located using debuginfod, search locally */
- if (fd < 0)
-fd = open(file_path.c_str(), O_RDONLY);
+if (fd < 0)
+ fd = open(file_path.c_str(), O_RDONLY);
if (fd < 0)
{
if (verbose)
@@ -371,11 +372,11 @@ void zip_files()
missing_files++;
if (verbose)
cerr << "Error: Failed to get file status for " << file_path << ": "
<< strerror(errno) << endl;
- continue;
+ goto next;
}
-struct archive_entry *entry = archive_entry_new();
+entry = archive_entry_new();
/* Removing first "/"" to make the path "relative" before zipping,
otherwise warnings are raised when unzipping. */
-string entry_name = file_path.substr(file_path.find_first_of('/') + 1);
+entry_name = file_path.substr(file_path.find_first_of('/') + 1);
archive_entry_set_pathname(entry, entry_name.c_str());
archive_entry_copy_stat(entry, &st);
if (archive_write_header(a, entry) != ARCHIVE_OK)
@@ -385,7 +386,7 @@ void zip_files()
missing_files++;
if (verbose)
cerr << "Error: failed to write header for " << file_path << ": " <<
archive_error_string(a) << endl;
- continue;
+ goto next;
}
/* Write the file to the zip. */
@@ -397,7 +398,7 @@ void zip_files()
missing_files++;
if (verbose)
cerr << "Error: Failed to open file: " << file_path << ": " <<
strerror(errno) < 0)
{
@@ -409,6 +410,8 @@ void zip_files()
}
len = read(fd, buff, sizeof(buff));
}
+
+next:
close(fd);
archive_entry_free(entry);
}
--
2.47.0
[PATCH] libdw: Don't use ATOMIC_VAR_INIT
ATOMIC_VAR_INIT was introduced in C11, but not deemed necessary to implement atomics. So deprecated in C17 and removed in C23. Normal initialization syntax should be sufficient. * libdw/libdw_alloc.c (next_id): Initialize to zero without using ATOMIC_VAR_INIT. Signed-off-by: Mark Wielaard --- libdw/libdw_alloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libdw/libdw_alloc.c b/libdw/libdw_alloc.c index b3e533434939..adc729f525f2 100644 --- a/libdw/libdw_alloc.c +++ b/libdw/libdw_alloc.c @@ -45,7 +45,7 @@ #define THREAD_ID_UNSET ((size_t) -1) static __thread size_t thread_id = THREAD_ID_UNSET; -static atomic_size_t next_id = ATOMIC_VAR_INIT(0); +static atomic_size_t next_id = 0; struct libdw_memblock * __libdw_alloc_tail (Dwarf *dbg) -- 2.47.0
[PATCH] Cleanup Makefile.am CLEANFILES definitions
Since b2f225d6bff8 ("Consolidate and add files to clean target variables")
autoreconf (automake) produces these warnings:
debuginfod/Makefile.am:130: warning: CLEANFILES multiply defined in condition
TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
debuginfod/Makefile.am:32: 'config/eu.am' included from here
libasm/Makefile.am:91: warning: CLEANFILES multiply defined in condition TRUE
...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libasm/Makefile.am:30: 'config/eu.am' included from here
libcpu/Makefile.am:105: warning: CLEANFILES multiply defined in condition TRUE
...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libcpu/Makefile.am:30: 'config/eu.am' included from here
libdw/Makefile.am:156: warning: CLEANFILES multiply defined in condition TRUE
...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libdw/Makefile.am:30: 'config/eu.am' included from here
libelf/Makefile.am:142: warning: CLEANFILES multiply defined in condition TRUE
...
config/eu.am:138: ... 'CLEANFILES' previously defined here
libelf/Makefile.am:30: 'config/eu.am' included from here
src/Makefile.am:47: warning: CLEANFILES multiply defined in condition TRUE ...
config/eu.am:138: ... 'CLEANFILES' previously defined here
src/Makefile.am:19: 'config/eu.am' included from here
tests/Makefile.am:891: warning: CLEANFILES multiply defined in condition TRUE
...
config/eu.am:138: ... 'CLEANFILES' previously defined here
tests/Makefile.am:19: 'config/eu.am' included from here
This is because config/eu.am defines a default CLEANFILES. So those
Makefile.am files should add to CLEANFILES with += instead of
redefining CLEANFILES with =.
* debuginfod/Makefile.am (CLEANFILES): Use +=.
* libasm/Makefile.am (CLEANFILES): Likewise.
* libcpu/Makefile.am (CLEANFILES): Likewise.
* libdw/Makefile.am (CLEANFILES): Likewise.
* libelf/Makefile.am (CLEANFILES): Likewise.
* src/Makefile.am (CLEANFILES): Likewise.
* tests/Makefile.am (CLEANFILES): Likewise.
Signed-off-by: Mark Wielaard
---
debuginfod/Makefile.am | 2 +-
libasm/Makefile.am | 2 +-
libcpu/Makefile.am | 2 +-
libdw/Makefile.am | 2 +-
libelf/Makefile.am | 2 +-
src/Makefile.am| 2 +-
tests/Makefile.am | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 45b5339f488c..1c35ae77c996 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -127,7 +127,7 @@ endif
EXTRA_DIST = libdebuginfod.map
MOSTLYCLEANFILES = $(am_libdebuginfod_a_OBJECTS)
$(am_libdebuginfod_pic_a_OBJECTS) $(LIBDEBUGINFOD_SONAME)
-CLEANFILES = libdebuginfod.so
+CLEANFILES += libdebuginfod.so
# automake std-options override: arrange to pass LD_LIBRARY_PATH
installcheck-binPROGRAMS: $(bin_PROGRAMS)
diff --git a/libasm/Makefile.am b/libasm/Makefile.am
index 324fd095a783..89650e675570 100644
--- a/libasm/Makefile.am
+++ b/libasm/Makefile.am
@@ -88,4 +88,4 @@ noinst_HEADERS = libasmP.h symbolhash.h
EXTRA_DIST = libasm.map
MOSTLYCLEANFILES = $(am_libasm_a_OBJECTS) $(am_libasm_pic_a_OBJECTS)
libasm.so.$(VERSION)
-CLEANFILES = libasm.so
+CLEANFILES += libasm.so
diff --git a/libcpu/Makefile.am b/libcpu/Makefile.am
index 3283523781c0..c70b988f7e48 100644
--- a/libcpu/Makefile.am
+++ b/libcpu/Makefile.am
@@ -102,5 +102,5 @@ bpf_disasm_CFLAGS = -Wno-format-nonliteral
EXTRA_DIST = defs/i386
MOSTLYCLEANFILES = $(am_libcpu_a_OBJECTS) $(am_libcpu_pic_a_OBJECTS)
$(i386_gendis_OBJECTS)
-CLEANFILES = $(foreach P,i386 x86_64,$P_defs $P.mnemonics)
+CLEANFILES += $(foreach P,i386 x86_64,$P_defs $P.mnemonics)
MAINTAINERCLEANFILES = $(foreach P,i386 x86_64, $P_defs $P_dis.h $P_parse.h)
diff --git a/libdw/Makefile.am b/libdw/Makefile.am
index 62f4359e4c09..42b18ce23e85 100644
--- a/libdw/Makefile.am
+++ b/libdw/Makefile.am
@@ -153,5 +153,5 @@ noinst_HEADERS = libdwP.h memory-access.h
dwarf_abbrev_hash.h \
EXTRA_DIST = libdw.map
MOSTLYCLEANFILES = $(am_libdw_a_OBJECTS) $(am_libdw_pic_a_OBJECTS)
libdw.so.$(VERSION)
-CLEANFILES = libdw.so
+CLEANFILES += libdw.so
MAINTAINERCLEANFILES = $(srcdir)/known-dwarf.h
diff --git a/libelf/Makefile.am b/libelf/Makefile.am
index e91bcd6e9ba1..603f797a3aa2 100644
--- a/libelf/Makefile.am
+++ b/libelf/Makefile.am
@@ -139,4 +139,4 @@ uninstall: uninstall-am
EXTRA_DIST = libelf.map
MOSTLYCLEANFILES = $(am_libelf_a_OBJECTS) $(am_libelf_pic_a_OBJECTS)
libelf.so.$(VERSION)
-CLEANFILES = libelf.so
+CLEANFILES += libelf.so
diff --git a/src/Makefile.am b/src/Makefile.am
index 97a0c61aac59..4221d54e4e77 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -44,7 +44,7 @@ bin_SCRIPTS = make-debug-archive
EXTRA_DIST = arlib.h debugpred.h make-debug-archive.in
MOSTLYCLEANFILES = *.gconv
-CLEANFILES = $(bin_SCRIPTS)
+CLEANFILES += $(bin_SCRIPTS)
if BUILD_STATIC
libasm = ../libasm/libasm.a
diff --git a/tests/Makefile.am b/tests/Makefile.am
in
Re: [PATCH] Cleanup Makefile.am CLEANFILES definitions
On Mon, Nov 18, 2024 at 1:12 PM Mark Wielaard wrote:
>
> Since b2f225d6bff8 ("Consolidate and add files to clean target variables")
> autoreconf (automake) produces these warnings:
>
> debuginfod/Makefile.am:130: warning: CLEANFILES multiply defined in condition
> TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> debuginfod/Makefile.am:32: 'config/eu.am' included from here
> libasm/Makefile.am:91: warning: CLEANFILES multiply defined in condition TRUE
> ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libasm/Makefile.am:30: 'config/eu.am' included from here
> libcpu/Makefile.am:105: warning: CLEANFILES multiply defined in condition
> TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libcpu/Makefile.am:30: 'config/eu.am' included from here
> libdw/Makefile.am:156: warning: CLEANFILES multiply defined in condition TRUE
> ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libdw/Makefile.am:30: 'config/eu.am' included from here
> libelf/Makefile.am:142: warning: CLEANFILES multiply defined in condition
> TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> libelf/Makefile.am:30: 'config/eu.am' included from here
> src/Makefile.am:47: warning: CLEANFILES multiply defined in condition TRUE ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> src/Makefile.am:19: 'config/eu.am' included from here
> tests/Makefile.am:891: warning: CLEANFILES multiply defined in condition TRUE
> ...
> config/eu.am:138: ... 'CLEANFILES' previously defined here
> tests/Makefile.am:19: 'config/eu.am' included from here
>
> This is because config/eu.am defines a default CLEANFILES. So those
> Makefile.am files should add to CLEANFILES with += instead of
> redefining CLEANFILES with =.
>
>* debuginfod/Makefile.am (CLEANFILES): Use +=.
>* libasm/Makefile.am (CLEANFILES): Likewise.
>* libcpu/Makefile.am (CLEANFILES): Likewise.
>* libdw/Makefile.am (CLEANFILES): Likewise.
>* libelf/Makefile.am (CLEANFILES): Likewise.
>* src/Makefile.am (CLEANFILES): Likewise.
>* tests/Makefile.am (CLEANFILES): Likewise.
Thanks Mark, this patch fixes the warnings for me.
Aaron
Re: [PATCH] Cleanup Makefile.am CLEANFILES definitions
Hi Mark,
> Since b2f225d6bff8 ("Consolidate and add files to clean target variables")
> autoreconf (automake) produces these warnings:
oops, sorry...
> This is because config/eu.am defines a default CLEANFILES. So those
> Makefile.am files should add to CLEANFILES with += instead of
> redefining CLEANFILES with =.
May I suggest to instead simply use
"DISTCLEANFILES" for what is in eu.am?
It's not being used anywhere else currently...
If that's no good, I would then like to see a '+='
in all the "*CLEANFILES" definitions in each level for each file
so it's not confusing or looking like a possible mistake.
--
MCP
Re: [PATCH] libdw: Don't use ATOMIC_VAR_INIT
Hi Mark, On Mon, Nov 18, 2024 at 1:56 PM Mark Wielaard wrote: > > ATOMIC_VAR_INIT was introduced in C11, but not deemed necessary to > implement atomics. So deprecated in C17 and removed in C23. Normal > initialization syntax should be sufficient. > > * libdw/libdw_alloc.c (next_id): Initialize to zero without > using ATOMIC_VAR_INIT. > > Signed-off-by: Mark Wielaard > --- > libdw/libdw_alloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libdw/libdw_alloc.c b/libdw/libdw_alloc.c > index b3e533434939..adc729f525f2 100644 > --- a/libdw/libdw_alloc.c > +++ b/libdw/libdw_alloc.c > @@ -45,7 +45,7 @@ > > #define THREAD_ID_UNSET ((size_t) -1) > static __thread size_t thread_id = THREAD_ID_UNSET; > -static atomic_size_t next_id = ATOMIC_VAR_INIT(0); > +static atomic_size_t next_id = 0; > > struct libdw_memblock * > __libdw_alloc_tail (Dwarf *dbg) > -- > 2.47.0 > LGTM. Aaron Ballman has noted [1] that no implementation of atomics or embedded locks has depended on the use of this macro. Aaron [1] https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1138r0.pdf
