<div dir="ltr">Thanks, Rafael. I committed this patch as r186623.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 18, 2013 at 1:40 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I would still prefer to not push forward with the unnecessary use of<br>
error_code, but Michael's review is certainly sufficient.<br>
<div class="HOEnZb"><div class="h5"><br>
On 18 July 2013 16:16, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
> It's not very clear to me if this patch is stamped. Rafael, are you ok with<br>
> this patch?<br>
><br>
><br>
> On Tue, Jul 16, 2013 at 3:28 PM, Michael Spencer <<a href="mailto:bigcheesegs@gmail.com">bigcheesegs@gmail.com</a>><br>
> wrote:<br>
>><br>
>> On Tue, Jul 16, 2013 at 2:52 PM, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
>>><br>
>>> Actually this code is outside of lld and was written mainly by Michael.<br>
>>> I'd like to get his input about API rework. If he's okay with API rework I'm<br>
>>> fine to work on that. However, what I'm trying to do is to add a relatively<br>
>>> small method to the set of methods which spans multiple files in a<br>
>>> consistent manner, so I'm tempted to say API rework shouldn't block this<br>
>>> change.<br>
>>><br>
>>><br>
>>> On Tue, Jul 16, 2013 at 2:41 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>><br>
>>>> On Tue, Jul 16, 2013 at 2:37 PM, Rafael Espíndola<br>
>>>> <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
>>>>><br>
>>>>> > I agree with you in general but in this case consistency looks more<br>
>>>>> > important than that. Look at the other methods, such as the above<br>
>>>>> > two.<br>
>>>>> > getCOFFHeader() and getPE32Header() will never fail and will always<br>
>>>>> > return<br>
>>>>> > object_error::success. Other methods in the other files in the same<br>
>>>>> > directory are written in the similar manner. If we change the<br>
>>>>> > signature of<br>
>>>>> > this function, it won't fix the API, but it'd make the API cluttered.<br>
>>>>> ><br>
>>>>> > I'd think we should change all the other methods in the directory not<br>
>>>>> > to<br>
>>>>> > return error_code, if we think it's the right thing to do. That<br>
>>>>> > should be<br>
>>>>> > done in another patch.<br>
>>>>><br>
>>>>> I think we should change it all, yes. If you don't want to have the<br>
>>>>> methods temporarily in a mixed state, then please change the existing<br>
>>>>> ones first.<br>
>>>><br>
>>>><br>
>>>> Rafael, I don't think that's necessary. This is all code that Rui has<br>
>>>> written and been maintaining. If he would prefer to do the API rework in a<br>
>>>> follow-up patch that seems totally fine.<br>
>>><br>
>>><br>
>><br>
>> Error handling can be fixed separately. It's a problem in all of<br>
>> libObject.<br>
>><br>
>> lgtm.<br>
>><br>
>> - Michael Spencer<br>
>><br>
><br>
</div></div></blockquote></div><br></div>