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

Tedeschi, Walfred
2017-09-18T16:34:53Z

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