This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [GOLD][PATCH] Add Arm_output_section class and downcasting hooks.
I committed the patch with changes you suggested. I also added
asserts to make sure that group_end is never the end of the container.
The asserts did not trigger in my testing. So I think the original
code was correct but I change it anyway. The assignment of
container.end() to group_end is mainly for eliminating possible
unitialized variable warnings. When a new stub-group is beind built,
group_end never points to container.end().
As for the static casts, I don't think I can reliably detects the
subclasses of Output_section, Output_relaxed_input_section and
Sized_relobj by checking the machine code and endianity of the target.
At the moment it works because we always use these subclasses instead
of the bases so I do not worry too mcuh about that.
-Doug
2009/10/23 Doug Kwan (Ãö®¶¼w) <dougkwan@google.com>:
> The range is meant to be inclusive, if I sometimes pass container.end(),
> then it is a bug. I will fix that may be with a do while loop.
>
> Originally I used static casts, they are actually safe because of context.
> In the ARM back-end, we always use the target subclass of Output_object for
> example. I will change that back to static casts with some comments
> explaining why they are safe then.
>
> Thanks for looking at the patch.
>
> On Oct 23, 2009 6:44 PM, "Ian Lance Taylor" <iant@google.com> wrote:
>
> "Doug Kwan (Ãö®¶¼w)" <dougkwan@google.com> writes: >> 2009-10-22 Doug Kwan
> <dougkwan@google.com> >> ...
>
>> + Input_section_list::const_iterator after_end = end + 1;
>> + for (Input_section_list::const_iterator p = begin; p != after_end; ++p)
>
> This is not the convention for C++ iterators. The convention is p =
> begin; p != end; ++p, without using after_end. Moreover, I see that
> later on you do sometimes pass CONTAINER.end() as the end parameter
> here, in which case end + 1 is undefined.
>
>
>
>> + // Return a pointer to self if this is an Arm_relobj or NULL otherwise.
>> + template<bool big_endian>
>> + Arm_relobj<big_endian>*
>> + arm_relobj()
>> + {
>> + Arm_relobj<big_endian>* as_arm_relobj;
>> + this->do_arm_relobj(&as_arm_relobj);
>> + return as_arm_relobj;
>> + }
>
> This is nicely type safe but I don't think we want target specific
> stuff getting into object.h/object.cc. Just use a static_cast in
> arm.cc. If you want to make it safe, do something which verifies that
> machine_code(), get_size(), and is_big_endian() are what you expect.
>
>
> This is OK with these changes if you only change arm.cc. If
> static_cast doesn't do what you need, let's take another round.
>
> Ian
>