[PATCH] COFFDumper: Dump data directory entries.

Rui Ueyama ruiu at google.com
Thu Jul 18 13:16:59 PDT 2013


It's not very clear to me if this patch is stamped. Rafael, are you ok with
this patch?


On Tue, Jul 16, 2013 at 3:28 PM, Michael Spencer <bigcheesegs at gmail.com>wrote:

> On Tue, Jul 16, 2013 at 2:52 PM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Actually this code is outside of lld and was written mainly by Michael.
>> I'd like to get his input about API rework. If he's okay with API rework
>> I'm fine to work on that. However, what I'm trying to do is to add a
>> relatively small method to the set of methods which spans multiple files in
>> a consistent manner, so I'm tempted to say API rework shouldn't block this
>> change.
>>
>>
>> On Tue, Jul 16, 2013 at 2:41 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>
>>>
>>> On Tue, Jul 16, 2013 at 2:37 PM, Rafael EspĂ­ndola <
>>> rafael.espindola at gmail.com> wrote:
>>>
>>>> > I agree with you in general but in this case consistency looks more
>>>> > important than that. Look at the other methods, such as the above two.
>>>> > getCOFFHeader() and getPE32Header() will never fail and will always
>>>> return
>>>> > object_error::success. Other methods in the other files in the same
>>>> > directory are written in the similar manner. If we change the
>>>> signature of
>>>> > this function, it won't fix the API, but it'd make the API cluttered.
>>>> >
>>>> > I'd think we should change all the other methods in the directory not
>>>> to
>>>> > return error_code, if we think it's the right thing to do. That
>>>> should be
>>>> > done in another patch.
>>>>
>>>> I think we should change it all, yes. If you don't want to have the
>>>> methods temporarily in a mixed state, then please change the existing
>>>> ones first.
>>>
>>>
>>> Rafael, I don't think that's necessary. This is all code that Rui has
>>> written and been maintaining. If he would prefer to do the API rework in a
>>> follow-up patch that seems totally fine.
>>>
>>
>>
> Error handling can be fixed separately. It's a problem in all of libObject.
>
> lgtm.
>
> - Michael Spencer
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130718/c773d6eb/attachment.html>


More information about the llvm-commits mailing list