[PATCH] COFFDumper: Dump data directory entries.

[Rui Ueyama]

Thanks, Rafael. I committed this patch as r186623.


On Thu, Jul 18, 2013 at 1:40 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> I would still prefer to not push forward with the unnecessary use of
> error_code, but Michael's review is certainly sufficient.
>
> On 18 July 2013 16:16, Rui Ueyama <ruiu at google.com> wrote:
> > 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/f8a2e314/attachment.html>