bspatch: Report errors on MBS_ApplyPatch.

bspatch: Report errors on MBS_ApplyPatch.

ApplyBinaryPatch() returns an integer value different from 0 on
error when opening files. Nevertheless, it doesn't report the errors
when they happen on the underlying MBS_ApplyPatch() function.

BUG=None
TEST=Manually compiled and called ApplyBinaryPatch with an invalid patch.
Function returns error instead of 0.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=270266

[deymo, 2014-05-13 00:33:31]

Hi Greg, you seem to be the last one touching code that uses this mbspatch. I
saw this when reading the code and didn't want to ignore it. Thanks for review.

[grt (UTC plus 2), 2014-05-13 15:10:05]

https://codereview.chromium.org/284733002/diff/1/third_party/bspatch/mbspatch.cc
File third_party/bspatch/mbspatch.cc (right):

https://codereview.chromium.org/284733002/diff/1/third_party/bspatch/mbspatch...
third_party/bspatch/mbspatch.cc:266: ret = MBS_ApplyPatch(&header, pfd, buf,
nfd);
nice catch. shouldn't this be:
    if ((ret = MBS_ApplyPatch(&header, pfd, buf, nfd)))
      break;

[deymo, 2014-05-13 18:14:40]

https://codereview.chromium.org/284733002/diff/1/third_party/bspatch/mbspatch.cc
File third_party/bspatch/mbspatch.cc (right):

https://codereview.chromium.org/284733002/diff/1/third_party/bspatch/mbspatch...
third_party/bspatch/mbspatch.cc:266: ret = MBS_ApplyPatch(&header, pfd, buf,
nfd);
On 2014/05/13 15:10:06, grt wrote:
> nice catch. shouldn't this be:
>     if ((ret = MBS_ApplyPatch(&header, pfd, buf, nfd)))
>       break;

Yeah, both options work (because this is the last instruction on the 'do {}
while(0)' ). I can change that anyway.

[grt (UTC plus 2), 2014-05-13 18:37:04]

https://codereview.chromium.org/284733002/diff/1/third_party/bspatch/mbspatch.cc
File third_party/bspatch/mbspatch.cc (right):

https://codereview.chromium.org/284733002/diff/1/third_party/bspatch/mbspatch...
third_party/bspatch/mbspatch.cc:266: ret = MBS_ApplyPatch(&header, pfd, buf,
nfd);
On 2014/05/13 18:14:40, deymo wrote:
> On 2014/05/13 15:10:06, grt wrote:
> > nice catch. shouldn't this be:
> >     if ((ret = MBS_ApplyPatch(&header, pfd, buf, nfd)))
> >       break;
> 
> Yeah, both options work (because this is the last instruction on the 'do {}
> while(0)' ). I can change that anyway.

oh, ha. i didn't notice that it was while(0). in that case, lgtm.

[deymo, 2014-05-13 18:49:49]

> oh, ha. i didn't notice that it was while(0). in that case, lgtm.

Yeah, that's the nicer alternative to "goto fail" when you don't have nested
loops.

[deymo, 2014-05-13 18:50:05]

The CQ bit was checked by deymo@chromium.org

[commit-bot: I haz the power, 2014-05-13 18:51:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/deymo@chromium.org/284733002/1

[commit-bot: I haz the power, 2014-05-13 20:54:35]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-13 20:59:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-05-13 20:59:17]

Try jobs failed on following builders:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[deymo, 2014-05-13 21:25:12]

The CQ bit was checked by deymo@chromium.org

[commit-bot: I haz the power, 2014-05-13 21:28:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/deymo@chromium.org/284733002/1

[commit-bot: I haz the power, 2014-05-13 23:15:04]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/7...)

[commit-bot: I haz the power, 2014-05-14 00:27:24]

Change committed as 270266