Re: [PATCH] icc: allow code path for newer versions of icc.

["Tedeschi, Walfred" <walfred dot tedeschi at intel dot com>]

Hi Pedro and Simon,


Thanks again for the review!


On 09/18/2017 06:23 PM, Simon Marchi wrote:
> On 2017-09-18 17:34, Pedro Alves wrote:
> > Hi,
> >
> > I read the patch and I found a few nits to pick.
> > See below.
> >
> > On 09/06/2017 09:46 AM, Walfred Tedeschi wrote:
> > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> > > index 6678b33..4aa3b3e 100644
> > > --- a/gdb/dwarf2read.c
> > > +++ b/gdb/dwarf2read.c
> > > @@ -604,7 +604,7 @@ struct dwarf2_cu
> > >    unsigned int checked_producer : 1;
> >
> > ...
> >
> > > +#if defined GDB_SELF_TEST
> > > +#include "selftest.h"
> > > +
> > > +namespace selftests {
> > > +namespace gdbserver {
> > > +static void
> > > +dwarf_producer_test ()
> >
> > Simon already pointed at the gdbserver namespace.  I'd like
> > to add that it looks odd to me to define the actual
> > functionality in utils.c and define the corresponding self
> > tests in dwarf2read.c.  Why not put the tests in utils.c as well?
> >
> > BTW, I'd support moving all these producer checks to
> > a separate file (maybe producer.c), instead of putting
> > it all in the utils.c kitchen sync, though that's a
> > preexisting issue.
>
> I agree about that.  I almost wrote that this function didn't really 
> belong in utils.c, until I saw that it's where other similar functions 
> were already.
>
> Simon

I am considering sending then two patches:
1. Add things in utils.c
2. move all producers to a new producers.h/c file.

Would this be ok?

Thanks again,
/Fred
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Christian Lamprechter
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928