Hi Housam,
On Fri, Jan 26, 2024 at 4:01 PM Housam Alamour <halamour@redhat.com> wrote:
>
> +/* Definitions of arguments for argp functions. */
> static const struct argp_option options[] =
> {
> { NULL, 0, NULL, OPTION_DOC, N_("Output options:"), 1 },
> @@ -89,58 +89,50 @@ static const struct argp_option options[] =
> { "verbose", 'v', NULL, 0,
> N_ ("Increase verbosity of logging messages."), 0 },
> { "cu-only", 'c', NULL, 0, N_ ("Only list the CU names."), 0 },
> - #ifdef HAVE_LIBARCHIVE
> - { "zip", 'z', NULL, 0, N_ ("Zip all the source files and send to stdout."
> - "Cannot be used with the null option"), 0 },
> - #ifdef ENABLE_LIBDEBUGINFOD
> - { "no-backup", 'b', NULL, 0, N_ ("Disables local source file search when "
> - "debuginfod fails to fetch files. This option is only applicable"
> - "when fetching and zipping files."), 0 },
> - #endif
> - #endif
> + { "zip", 'z', NULL, 0, N_ ("Zip all the source files and send to stdout. "
> + "By default, priority is given to fetching source files from "
> + "debuginfod over local source files"), 0 },
> + { "no-backup", 'b', NULL, 0, N_ ("Disables local source file search when "
> + "debuginfod fails to fetch files. This option is only applicable "
> + "when fetching and zipping files."), 0 },
> { NULL, 0, NULL, 0, NULL, 0 }
> };
I originally suggested keeping -z/-b as valid options even when
srcfiles isn't built with libarchive/libdebuginfod support, but later on
IRC we decided to keep -z/-b conditional on building srcfiles with
the necessary support. I think these #ifdef might have been removed
by mistake.
Also when you post a revised patch, it's a bit easier for me if you just
post the current definitive patch with all commits squashed. I'll just diff
your patches if I want to compare changes since the last revision!
Aaron
@@ -83,9 +83,16 @@ Print program version.
\fB\-0, \-\-null\fR
Separate items by a null instead of a newline.
+.TP
+\fB\-b, \-\-no-backup\fR
+Disables local source file search when
+debuginfod fails to fetch files. This
+option is only applicable when fetching and
+zipping files.
+
.TP
\fB\-c, \-\-cu\-only\fR
-Only list the (compilation unit) CU names.
+Only list the CU (compilation unit) names.
.TP
\fB\-v, \-\-verbose\fR
@@ -94,7 +101,7 @@ Increase verbosity of logging messages.
.TP
\fB\-z, \-\-zip\fR
Zip all the source files and send to stdout.
-Files may be automatically fetched by
+By default, files will be automatically fetched by
debuginfod (if applicable) or locally as a
backup. Any source files that were not found
will not be archived.
@@ -20,11 +20,11 @@
/* In case we have a bad fts we include this before config.h because it
can't handle _FILE_OFFSET_BITS.
Everything we need here is fine if its declarations just come first.
- Also, include sys/types.h before fts. On some systems fts.h is not self
- contained. */
+ Also, include sys/types.h before fts. On some systems fts.h is not self
+ contained. */
#ifdef BAD_FTS
- #include <sys/types.h>
- #include <fts.h>
+#include <sys/types.h>
+#include <fts.h>
#endif
#ifdef HAVE_CONFIG_H
@@ -42,7 +42,7 @@
#include <memory>
#ifdef ENABLE_LIBDEBUGINFOD
- #include "debuginfod.h"
+#include "debuginfod.h"
#endif
#include <libdwfl.h>
@@ -54,17 +54,17 @@
/* Libraries for use by the --zip option */
#ifdef HAVE_LIBARCHIVE
- #include <archive.h>
- #include <archive_entry.h>
+#include <archive.h>
+#include <archive_entry.h>
#endif
/* If fts.h is included before config.h, its indirect inclusions may not
- give us the right LFS aliases of these functions, so map them manually. */
+ give us the right LFS aliases of these functions, so map them manually. */
#ifdef BAD_FTS
- #ifdef _FILE_OFFSET_BITS
- #define open open64
- #define fopen fopen64
- #endif
+#ifdef _FILE_OFFSET_BITS
+#define open open64
+#define fopen fopen64
+#endif
#else
#include <sys/types.h>
#include <fts.h>
@@ -72,15 +72,15 @@
using namespace std;
-/* Name and version of program. */
+/* Name and version of program. */
ARGP_PROGRAM_VERSION_HOOK_DEF = print_version;
-/* Bug report address. */
+/* Bug report address. */
ARGP_PROGRAM_BUG_ADDRESS_DEF = PACKAGE_BUGREPORT;
constexpr size_t BUFFER_SIZE = 8192;
-/* Definitions of arguments for argp functions. */
+/* Definitions of arguments for argp functions. */
static const struct argp_option options[] =
{
{ NULL, 0, NULL, OPTION_DOC, N_("Output options:"), 1 },
@@ -89,58 +89,50 @@ static const struct argp_option options[] =
{ "verbose", 'v', NULL, 0,
N_ ("Increase verbosity of logging messages."), 0 },
{ "cu-only", 'c', NULL, 0, N_ ("Only list the CU names."), 0 },
- #ifdef HAVE_LIBARCHIVE
- { "zip", 'z', NULL, 0, N_ ("Zip all the source files and send to stdout."
- "Cannot be used with the null option"), 0 },
- #ifdef ENABLE_LIBDEBUGINFOD
- { "no-backup", 'b', NULL, 0, N_ ("Disables local source file search when "
- "debuginfod fails to fetch files. This option is only applicable"
- "when fetching and zipping files."), 0 },
- #endif
- #endif
+ { "zip", 'z', NULL, 0, N_ ("Zip all the source files and send to stdout. "
+ "By default, priority is given to fetching source files from "
+ "debuginfod over local source files"), 0 },
+ { "no-backup", 'b', NULL, 0, N_ ("Disables local source file search when "
+ "debuginfod fails to fetch files. This option is only applicable "
+ "when fetching and zipping files."), 0 },
{ NULL, 0, NULL, 0, NULL, 0 }
};
-/* Short description of program. */
-static const char doc[] = N_("Lists the source files of a DWARF/ELF file. The default input is the file 'a.out'.");
+/* Short description of program. */
+static const char doc[] = N_("Lists the source files of a DWARF/ELF file. The default input is the file 'a.out'.");
-/* Strings for arguments in help texts. */
+/* Strings for arguments in help texts. */
static const char args_doc[] = N_("INPUT");
-/* Prototype for option handler. */
+/* Prototype for option handler. */
static error_t parse_opt (int key, char *arg, struct argp_state *state);
-static struct argp_child argp_children[2]; /* [0] is set in main. */
+static struct argp_child argp_children[2]; /* [0] is set in main. */
-/* Data structure to communicate with argp functions. */
+/* Data structure to communicate with argp functions. */
static const struct argp argp =
{
options, parse_opt, args_doc, doc, argp_children, NULL, NULL
};
-/* Verbose message printing. */
+/* Verbose message printing. */
static bool verbose;
-/* Delimit the output with nulls. */
+/* Delimit the output with nulls. */
static bool null_arg;
-/* Only print compilation unit names. */
+/* Only print compilation unit names. */
static bool CU_only;
-#ifdef HAVE_LIBARCHIVE
- /* Zip all the source files and send to stdout. */
- static bool zip;
-
- #ifdef ENABLE_LIBDEBUGINFOD
- /* Disables local source file search when debuginfod fails to fetch them.
- This option is only applicable when fetching and zipping files.*/
- static bool no_backup;
- #endif
-#endif
-
-/* Handle program arguments. Note null arg and zip
- cannot be combined due to warnings raised when unzipping. */
+/* Zip all the source files and send to stdout. */
+static bool zip;
+/* Disables local source file search when debuginfod fails to fetch them.
+ This option is only applicable when fetching and zipping files.*/
+static bool no_backup;
+
+/* Handle program arguments. Note null arg and zip
+ cannot be combined due to warnings raised when unzipping. */
static error_t
parse_opt (int key, char *arg, struct argp_state *state)
{
- /* Suppress "unused parameter" warning. */
+ /* Suppress "unused parameter" warning. */
(void)arg;
switch (key)
{
@@ -179,18 +171,18 @@ parse_opt (int key, char *arg, struct argp_state *state)
}
/* Remove the "/./" , "../" and the preceding directory
- that some paths include which raise errors during unzip. */
+ that some paths include which raise errors during unzip. */
string canonicalize_path(string path)
{
stringstream ss(path);
string token;
vector<string> tokens;
- /* Extract each directory of the path and place into a vector. */
+ /* Extract each directory of the path and place into a vector. */
while (getline(ss, token, '/')) {
- /* Ignore any empty //, or /./ dirs. */
+ /* Ignore any empty //, or /./ dirs. */
if (token == "" || token == ".")
continue;
- /* When /.. is encountered, remove the most recent directory from the vector. */
+ /* When /.. is encountered, remove the most recent directory from the vector. */
else if (token == "..") {
if (!tokens.empty())
tokens.pop_back();
@@ -200,17 +192,17 @@ string canonicalize_path(string path)
stringstream result;
if (tokens.empty())
return "/";
- /* Reconstruct the path from the extracted directories. */
+ /* Reconstruct the path from the extracted directories. */
for (const string &t : tokens) {
result << '/' << t;
}
return result.str();
}
-/* Global list of collected source files and their respective module.
+/* Global list of collected source files and their respective module.
Normally, it'll contain the sources of just one named binary, but
the '-K' option can cause multiple dwfl modules to be loaded, thus
- listed. */
+ listed. */
set<pair<string, Dwfl_Module*>> debug_sourcefiles;
static int
@@ -221,14 +213,14 @@ collect_sourcefiles (Dwfl_Module *dwflmod,
void *arg __attribute__ ((unused)))
{
Dwarf *dbg;
- Dwarf_Addr bias; /* ignored - for addressing purposes only. */
+ Dwarf_Addr bias; /* ignored - for addressing purposes only. */
dbg = dwfl_module_getdwarf (dwflmod, &bias);
Dwarf_Off offset = 0;
Dwarf_Off old_offset;
size_t hsize;
- /* Traverse all CUs of this module. */
+ /* Traverse all CUs of this module. */
while (dwarf_nextcu (dbg, old_offset = offset, &offset, &hsize, NULL, NULL, NULL) == 0)
{
Dwarf_Die cudie_mem;
@@ -243,7 +235,7 @@ collect_sourcefiles (Dwfl_Module *dwflmod,
if (dwarf_getsrcfiles (cudie, &files, &nfiles) != 0)
continue;
- /* extract DW_AT_comp_dir to resolve relative file names. */
+ /* extract DW_AT_comp_dir to resolve relative file names. */
const char *comp_dir = "";
const char *const *dirs;
size_t ndirs;
@@ -261,7 +253,7 @@ collect_sourcefiles (Dwfl_Module *dwflmod,
if (comp_dir[0] == '\0' && cuname[0] != '/')
{
/* This is a common symptom for dwz-compressed debug files,
- where the altdebug file cannot be resolved. */
+ where the altdebug file cannot be resolved. */
if (verbose)
clog << "skipping cu=" << cuname << " due to empty comp_dir" << endl;
continue;
@@ -304,112 +296,118 @@ collect_sourcefiles (Dwfl_Module *dwflmod,
}
#ifdef HAVE_LIBARCHIVE
- void zip_files()
+void zip_files()
+{
+ struct archive *a = archive_write_new();
+ struct stat st;
+ char buff[BUFFER_SIZE];
+ int len;
+ int fd;
+ #ifdef ENABLE_LIBDEBUGINFOD
+ /* Initialize a debuginfod client. */
+ static unique_ptr <debuginfod_client, void (*)(debuginfod_client*)>
+ client (debuginfod_begin(), &debuginfod_end);
+ #endif
+
+ archive_write_set_format_zip(a);
+ archive_write_open_fd(a, STDOUT_FILENO);
+
+ int missing_files = 0;
+ for (const auto &pair : debug_sourcefiles)
{
- struct archive *a = archive_write_new();
- struct stat st;
- char buff[BUFFER_SIZE];
- int len;
- int fd;
+ fd = -1;
+ const std::string &file_path = pair.first;
+
+ /* Attempt to query debuginfod client to fetch source files. */
#ifdef ENABLE_LIBDEBUGINFOD
- /* Initialize a debuginfod client. */
- static unique_ptr <debuginfod_client, void (*)(debuginfod_client*)>
- client (debuginfod_begin(), &debuginfod_end);
+ Dwfl_Module* dwflmod = pair.second;
+ /* Obtain source file's build ID. */
+ const unsigned char *bits;
+ GElf_Addr vaddr;
+ int bits_length = dwfl_module_build_id(dwflmod, &bits, &vaddr);
+ /* Ensure successful client and build ID acquisition. */
+ if (client.get() != NULL && bits_length > 0)
+ {
+ fd = debuginfod_find_source(client.get(),
+ bits, bits_length,
+ file_path.c_str(), NULL);
+ }
+ else
+ {
+ if (client.get() == NULL)
+ cerr << "Error: Failed to initialize debuginfod client." << endl;
+ else
+ cerr << "Error: Invalid build ID length (" << bits_length << ")." << endl;
+ }
#endif
- archive_write_set_format_zip(a);
- archive_write_open_fd(a, STDOUT_FILENO);
-
- int missing_files = 0;
- for (const auto &pair : debug_sourcefiles)
+ if (!no_backup)
+ /* Files could not be located using debuginfod, search locally */
+ if (fd < 0)
+ fd = open(file_path.c_str(), O_RDONLY);
+ if (fd < 0)
{
- fd = -1;
- const std::string &file_path = pair.first;
+ if (verbose)
+ cerr << file_path << endl;
+ missing_files++;
+ continue;
+ }
- /* Attempt to query debuginfod client to fetch source files. */
- #ifdef ENABLE_LIBDEBUGINFOD
- Dwfl_Module* dwflmod = pair.second;
- /* Obtain source file's build ID. */
- const unsigned char *bits;
- GElf_Addr vaddr;
- int bits_length = dwfl_module_build_id(dwflmod, &bits, &vaddr);
- /* Ensure successful client and build ID acquisition. */
- if (client.get() != NULL && bits_length > 0)
- {
- fd = debuginfod_find_source(client.get(),
- bits, bits_length,
- file_path.c_str(), NULL);
- }
- else
- {
- if (client.get() == NULL)
- cerr << "Error: Failed to initialize debuginfod client." << endl;
- else
- cerr << "Error: Invalid build ID length (" << bits_length << ")." << endl;
- }
-
- #endif
- if (!no_backup)
- /* Files could not be located using debuginfod, search locally */
- if (fd < 0)
- fd = open(file_path.c_str(), O_RDONLY);
- if (fd < 0)
- {
- missing_files++;
- continue;
- }
+ /* Create an entry for each file including file information to be placed in the zip. */
+ if (fstat(fd, &st) == -1)
+ {
+ if (verbose)
+ cerr << file_path << endl;
+ missing_files++;
+ if (verbose)
+ cerr << "Error: Failed to get file status for " << file_path << ": " << strerror(errno) << endl;
+ continue;
+ }
+ struct archive_entry *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);
+ archive_entry_set_pathname(entry, entry_name.c_str());
+ archive_entry_copy_stat(entry, &st);
+ if (archive_write_header(a, entry) != ARCHIVE_OK)
+ {
+ if (verbose)
+ cerr << file_path << endl;
+ missing_files++;
+ if (verbose)
+ cerr << "Error: failed to write header for " << file_path << ": " << archive_error_string(a) << endl;
+ continue;
+ }
- /* Create an entry for each file including file information to be placed in the zip. */
- if (fstat(fd, &st) == -1)
- {
- missing_files++;
- if (verbose)
- cerr << "Error: Failed to get file status for " << file_path << ": " << strerror(errno) << endl;
- continue;
- }
- struct archive_entry *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);
- archive_entry_set_pathname(entry, entry_name.c_str());
- archive_entry_copy_stat(entry, &st);
- if (archive_write_header(a, entry) != ARCHIVE_OK)
+ /* Write the file to the zip. */
+ len = read(fd, buff, sizeof(buff));
+ if (len == -1)
+ {
+ if (verbose)
+ cerr << file_path << endl;
+ missing_files++;
+ if (verbose)
+ cerr << "Error: Failed to open file: " << file_path << ": " << strerror(errno) <<endl;
+ continue;
+ }
+ while (len > 0)
+ {
+ if (archive_write_data(a, buff, len) < ARCHIVE_OK)
{
- missing_files++;
if (verbose)
- cerr << "Error: failed to write header for " << file_path << ": " << archive_error_string(a) << endl;
- continue;
+ cerr << "Error: Failed to read from the file: " << file_path << ": " << strerror(errno) << endl;
+ break;
}
-
- /* Write the file to the zip. */
len = read(fd, buff, sizeof(buff));
- if (len == -1)
- {
- missing_files++;
- if (verbose)
- cerr << "Error: Failed to open file: " << file_path << ": " << strerror(errno) <<endl;
- continue;
- }
- if (verbose && len > 0)
- clog << "Writing to zip: " << file_path << endl;
- while (len > 0)
- {
- if (archive_write_data(a, buff, len) < ARCHIVE_OK)
- {
- if (verbose)
- cerr << "Error: Failed to read from the file: " << file_path << ": " << strerror(errno) << endl;
- break;
- }
- len = read(fd, buff, sizeof(buff));
- }
- close(fd);
- archive_entry_free(entry);
}
- if (verbose && missing_files > 0 )
- cerr << "WARNING: " << missing_files << " files could not be found." << endl;
-
- archive_write_close(a);
- archive_write_free(a);
+ close(fd);
+ archive_entry_free(entry);
}
+ if (verbose && missing_files > 0 )
+ cerr << missing_files << " file(s) listed above could not be found. " << endl;
+
+ archive_write_close(a);
+ archive_write_free(a);
+}
#endif
int
@@ -417,14 +415,14 @@ main (int argc, char *argv[])
{
int remaining;
- /* Parse and process arguments. This includes opening the modules. */
+ /* Parse and process arguments. This includes opening the modules. */
argp_children[0].argp = dwfl_standard_argp ();
argp_children[0].group = 1;
Dwfl *dwfl = NULL;
(void) argp_parse (&argp, argc, argv, 0, &remaining, &dwfl);
assert (dwfl != NULL);
- /* Process all loaded modules - probably just one, except if -K or -p is used. */
+ /* Process all loaded modules - probably just one, except if -K or -p is used. */
(void) dwfl_getmodules (dwfl, &collect_sourcefiles, NULL, 0);
if (!debug_sourcefiles.empty ())
@@ -17,9 +17,6 @@
. $srcdir/debuginfod-subr.sh
-# prereqs
-type unzip 2>/dev/null || exit 77
-
# for test case debugging, uncomment:
# set -x
set -e
@@ -37,7 +34,7 @@ testrun $ET_EXEC -e $ET_EXEC | grep $SRC_NAME > /dev/null
# Check if zip option is available (only available if libarchive is available.
# Debuginfod optional to fetch source files from debuginfod federation.)
-$ET_EXEC --help | grep -q zip && zip=true || zip=false
+$ET_EXEC --help | grep -q zip && command -v unzip >/dev/null 2>&1 && zip=true || zip=false
for null_arg in --null ""; do
for verbose_arg in --verbose ""; do