Re: [PATCH] elfclassify tool

2019-08-13 Thread Mark Wielaard
Hi, With the dwelf_elf_begin fix now committed I intend to commit the elfclassify tool as attached. It now treats a NULL return from [dwelf_]elf_begin always as error. It has the new --library classification and various fixes that were pointed out during the review. Cheers, Mark>From

Re: [PATCH] elfclassify tool

2019-08-13 Thread Mark Wielaard
On Mon, Jul 29, 2019 at 04:38:17PM +0200, Florian Weimer wrote: > > On Mon, Jul 29, 2019 at 11:16:31AM +0200, Florian Weimer wrote: > > So the test should be: > > > > diff --git a/src/elfclassify.c b/src/elfclassify.c > > index ebd42c1d5..b17d14d45 100644 > > --- a/src/elfclassify.c > > +++

Re: [PATCH] elfclassify tool

2019-08-12 Thread Florian Weimer
* Mark Wielaard: > What do you think about this change to dwelf_elf_begin? > The change would make it possible to detect real errors in the > elfclassify code, whether elf_begin or dwelf_elf_begin was used. So we > would not misclassify files (but return an error status of 2). I'm not really

Re: [PATCH] elfclassify tool

2019-08-11 Thread Mark Wielaard
On Mon, 2019-07-29 at 16:24 +0200, Mark Wielaard wrote: > On Mon, Jul 29, 2019 at 10:43:56AM +0200, Florian Weimer wrote: > > * Mark Wielaard: > > > > > + if (elf == NULL) > > > +{ > > > + /* This likely means it just isn't an ELF file, probably not a > > > + real issue, but warn if

Re: [PATCH] elfclassify tool

2019-07-29 Thread Florian Weimer
* Mark Wielaard: >> Regarding the test case, I think if the build target is ELF, it makes >> sense to check that the elfutils binaries themselves are classified as >> expected, with the current build flags. This will detect changes >> required due to the evolution of the toolchain. > > That is

Re: [PATCH] elfclassify tool

2019-07-29 Thread Mark Wielaard
On Mon, Jul 29, 2019 at 11:22:13AM +0200, Florian Weimer wrote: > * Mark Wielaard: > > > Signed-off-by: Mark Wielaard > > Does elfutils use DCO? Then yoy have my signoff as well: > > Signed-off-by: Florian Weimer Thanks. Yes, elfutils uses a Developer Certificate of Origin based on the

Re: [PATCH] elfclassify tool

2019-07-29 Thread Florian Weimer
* Mark Wielaard: > On Mon, Jul 29, 2019 at 11:16:31AM +0200, Florian Weimer wrote: >> * Mark Wielaard: >> >> > +/* Called to process standard input if flag_stdin is not no_stdin. */ >> > +static void >> > +process_stdin (int *status) >> > +{ >> > + char delim; >> > + if (flag_stdin ==

Re: [PATCH] elfclassify tool

2019-07-29 Thread Mark Wielaard
On Mon, Jul 29, 2019 at 11:16:31AM +0200, Florian Weimer wrote: > * Mark Wielaard: > > > +/* Called to process standard input if flag_stdin is not no_stdin. */ > > +static void > > +process_stdin (int *status) > > +{ > > + char delim; > > + if (flag_stdin == do_stdin0) > > +delim = '\0'; >

Re: [PATCH] elfclassify tool

2019-07-29 Thread Mark Wielaard
On Mon, Jul 29, 2019 at 10:43:56AM +0200, Florian Weimer wrote: > * Mark Wielaard: > > > + if (elf == NULL) > > +{ > > + /* This likely means it just isn't an ELF file, probably not a > > +real issue, but warn if verbose reporting. */ > > + if (verbose > 0) > > + fprintf

Re: [PATCH] elfclassify tool

2019-07-29 Thread Florian Weimer
* Mark Wielaard: > Signed-off-by: Mark Wielaard Does elfutils use DCO? Then yoy have my signoff as well: Signed-off-by: Florian Weimer You should you list yourself as an author somewhere in the commit message. This is so much more than what I wrote. Regarding the test case, I think if the

Re: [PATCH] elfclassify tool

2019-07-29 Thread Florian Weimer
* Mark Wielaard: > +/* Called to process standard input if flag_stdin is not no_stdin. */ > +static void > +process_stdin (int *status) > +{ > + char delim; > + if (flag_stdin == do_stdin0) > +delim = '\0'; > + else > +delim = '\n'; > + > + char *buffer = NULL; > + size_t

Re: [PATCH] elfclassify tool

2019-07-29 Thread Florian Weimer
* Mark Wielaard: > + if (elf == NULL) > +{ > + /* This likely means it just isn't an ELF file, probably not a > + real issue, but warn if verbose reporting. */ > + if (verbose > 0) > + fprintf (stderr, "warning: %s: %s\n", current_path, elf_errmsg (-1)); > + return

Re: [PATCH] elfclassify tool

2019-07-27 Thread Mark Wielaard
On Sat, Jul 27, 2019 at 02:04:48AM +0300, Dmitry V. Levin wrote: > On Sat, Jul 20, 2019 at 11:51:16PM +0200, Mark Wielaard wrote: > > On Sat, Jul 20, 2019 at 01:57:27AM +0300, Dmitry V. Levin wrote: > [...] > > > btw, I think it would be appropriate to move the has_dynamic check before > > > the

Re: [PATCH] elfclassify tool

2019-07-26 Thread Dmitry V. Levin
On Sat, Jul 20, 2019 at 11:51:16PM +0200, Mark Wielaard wrote: > On Sat, Jul 20, 2019 at 01:57:27AM +0300, Dmitry V. Levin wrote: [...] > > btw, I think it would be appropriate to move the has_dynamic check before > > the first check in is_shared that returns true. > > Yes, that is probably fine,

Re: [PATCH] elfclassify tool

2019-07-26 Thread Mark Wielaard
Hi, On Mon, Jul 22, 2019 at 05:54:57PM +0200, Florian Weimer wrote: > I went through these patches, albeit in a somewhat cursory fashion, and > they look okay to me. > > Do you think this is enough to port over RPM's find-debuginfo.sh? Yes, I think this would make it possible to drop reliance

Re: [PATCH] elfclassify tool

2019-07-22 Thread Florian Weimer
* Mark Wielaard: > On Thu, 2019-04-18 at 13:17 +0200, Florian Weimer wrote: >> * Florian Weimer: >> >> > > BTW. Florian, the extra options are certainly not required for you to >> > > implement to get eu-elfclassify accepted. They are just suggestions, >> > > which we might decide not to do/add.

Re: [PATCH] elfclassify tool

2019-07-20 Thread Mark Wielaard
On Sat, Jul 20, 2019 at 01:57:27AM +0300, Dmitry V. Levin wrote: > On Fri, Jul 19, 2019 at 11:36:53PM +0200, Mark Wielaard wrote: > > > If the ELF type is ET_DYN and the object is not marked as DF_1_PIE, > > > could we come up with a more reliable heuristics than DT_SONAME and > > > PT_INTERP? >

Re: [PATCH] elfclassify tool

2019-07-20 Thread Mark Wielaard
On Fri, Jul 19, 2019 at 02:47:09PM +0200, Mark Wielaard wrote: > The individual commits can be found here: > https://code.wildebeest.org/git/user/mjw/elfutils/log/?h=elfclassify > > Please let me know if any of this looks bad or unusual. > > I'll write some testcases. And as always when writing

Re: [PATCH] elfclassify tool

2019-07-19 Thread Dmitry V. Levin
On Fri, Jul 19, 2019 at 11:36:53PM +0200, Mark Wielaard wrote: > On Sat, Jul 20, 2019 at 12:23:08AM +0300, Dmitry V. Levin wrote: > > On Fri, Jul 19, 2019 at 11:00:49PM +0200, Florian Weimer wrote: > > > * Dmitry V. Levin: > > > > > > >> So, I don't think the code is wrong. We might want to tweak

Re: [PATCH] elfclassify tool

2019-07-19 Thread Mark Wielaard
On Sat, Jul 20, 2019 at 12:23:08AM +0300, Dmitry V. Levin wrote: > On Fri, Jul 19, 2019 at 11:00:49PM +0200, Florian Weimer wrote: > > * Dmitry V. Levin: > > > > >> So, I don't think the code is wrong. We might want to tweak the comment > > >> a bit though, to make it less definitive? > > > > > >

Re: [PATCH] elfclassify tool

2019-07-19 Thread Dmitry V. Levin
On Fri, Jul 19, 2019 at 11:00:49PM +0200, Florian Weimer wrote: > * Dmitry V. Levin: > > >> So, I don't think the code is wrong. We might want to tweak the comment > >> a bit though, to make it less definitive? > > > > What I'm saying is that has_soname is just a hint which is probably even > >

Re: [PATCH] elfclassify tool

2019-07-19 Thread Florian Weimer
* Dmitry V. Levin: >> So, I don't think the code is wrong. We might want to tweak the comment >> a bit though, to make it less definitive? > > What I'm saying is that has_soname is just a hint which is probably even > less reliable than has_program_interpreter. If I recall correctly, I added the

Re: [PATCH] elfclassify tool

2019-07-19 Thread Dmitry V. Levin
On Fri, Jul 19, 2019 at 04:21:53PM +0200, Mark wrote: > On Fri, 2019-07-19 at 16:43 +0300, Dmitry V. Levin wrote: > > On Fri, Jul 19, 2019 at 02:47:09PM +0200, Mark Wielaard wrote: > > [...] > > > +static bool > > > +is_shared (void) > > > +{ > > > + if (!is_loadable ()) > > > +return false;

Re: [PATCH] elfclassify tool

2019-07-19 Thread Dmitry V. Levin
On Fri, Jul 19, 2019 at 02:47:09PM +0200, Mark Wielaard wrote: [...] > +static bool > +is_shared (void) > +{ > + if (!is_loadable ()) > +return false; > + > + /* The ELF type is very clear: this is an executable. */ > + if (elf_type == ET_EXEC) > +return false; > + > + /* If the

Re: [PATCH] elfclassify tool

2019-07-19 Thread Mark Wielaard
Hi, Some answers to this older discussion to explain some of my recent commits suggested for elfclassify. On Tue, 2019-04-16 at 13:38 +0200, Florian Weimer wrote: > * Mark Wielaard: > > --elf PATH return 0 whenever the file can be opened and a minimal ELF > > header can be read (it might not be

Re: [PATCH] elfclassify tool

2019-07-19 Thread Mark Wielaard
Hi, Sorry, this took way too long. But I really like this code. On Thu, 2019-04-18 at 13:17 +0200, Florian Weimer wrote: > * Florian Weimer: > > > > BTW. Florian, the extra options are certainly not required for you to > > > implement to get eu-elfclassify accepted. They are just suggestions, >

Re: [PATCH] elfclassify tool

2019-04-18 Thread Florian Weimer
* Florian Weimer: >> BTW. Florian, the extra options are certainly not required for you to >> implement to get eu-elfclassify accepted. They are just suggestions, >> which we might decide not to do/add. Or they can be added by others if >> they think they are useful. > > Understood. I would

Re: [PATCH] elfclassify tool

2019-04-16 Thread Florian Weimer
* Mark Wielaard: >> I still need to implement an --unstripped option and fix the >> iteration over the dynamic section. > > We did already discuss some of this off-list. Thanks for summarizing the previous discussion. > --elf PATH return 0 whenever the file can be opened and a minimal ELF >

Re: [PATCH] elfclassify tool

2019-04-15 Thread Mark Wielaard
Hi, On Fri, 2019-04-12 at 17:38 +0200, Florian Weimer wrote: > This patch adds an elfclassify tool, mainly for the benefit of RPM's > find-debuginfo.sh. I have CCed Panu to see if he has any input. > I still need to implement an --unstripped option and fix the > iteration over the dynamic