Avoid comparing size_t to be < 0 on AArch64.

Avoid comparing size_t to be < 0 on AArch64.

cpu_features_entries is empty on AArch64 and causes tautological-compare
warning when compiling with Clang.

BUG=539781

[Dmitry Skiba, 2016-01-06 22:00:29]

Description was changed from

==========
Avoid comparing size_t to be < 0 on AArch64.

cpu_features_entries is empty on AArch64 and causes tautological-compare
warning when compiling with Clang.

BUG=539781
==========

to

==========
Avoid comparing size_t to be < 0 on AArch64.

cpu_features_entries is empty on AArch64 and causes tautological-compare
warning when compiling with Clang.

BUG=539781
==========

[Dmitry Skiba, 2016-01-06 22:00:29]

dskiba@google.com changed reviewers:
+ thestig@chromium.org

[Dmitry Skiba, 2016-01-06 22:01:10]

Please review.

[Lei Zhang, 2016-01-06 22:06:21]

thestig@chromium.org changed reviewers:
+ mark@chromium.org

[Lei Zhang, 2016-01-06 22:06:22]

Mark, can we use range-based for loops in Breakpad? If so, the change can be
just:

for (const CpuFeaturesEntry& entry : cpu_features_entries) {
  if (tag_len == strlen(entry.tag) &&
      !memcmp(tag, entry.tag, tag_len)) { 
    sys_info->cpu.arm_cpu_info.elf_hwcaps |= entry.hwcaps;
    break;      
  }           
}

[Mark Mentovai, 2016-01-06 22:08:16]

The easiest way to know if we can use range-based for is to try it and go back
if it turns out that an important consumer can’t deal with it.

[Lei Zhang, 2016-01-06 22:12:40]

On 2016/01/06 22:08:16, Mark Mentovai wrote:
> The easiest way to know if we can use range-based for is to try it and go back
> if it turns out that an important consumer can’t deal with it.

Ok, let's do it and see what happens.

[Dmitry Skiba, 2016-01-06 22:13:49]

On 2016/01/06 22:08:16, Mark Mentovai wrote:
> The easiest way to know if we can use range-based for is to try it and go back
> if it turns out that an important consumer can’t deal with it.

BTW, there are 3 other places which look the same and can also be converted to
range loops. I believe the conversion should include those and be commit on its
own.

[Lei Zhang, 2016-01-06 22:32:25]

On 2016/01/06 22:13:49, Dmitry Skiba wrote:
> On 2016/01/06 22:08:16, Mark Mentovai wrote:
> > The easiest way to know if we can use range-based for is to try it and go
back
> > if it turns out that an important consumer can’t deal with it.
> 
> BTW, there are 3 other places which look the same and can also be converted to
> range loops. I believe the conversion should include those and be commit on
its
> own.

Feel free to fix them all in one go.

[Dmitry Skiba, 2016-01-06 23:33:14]

On 2016/01/06 22:32:25, Lei Zhang wrote:
> On 2016/01/06 22:13:49, Dmitry Skiba wrote:
> > On 2016/01/06 22:08:16, Mark Mentovai wrote:
> > > The easiest way to know if we can use range-based for is to try it and go
> back
> > > if it turns out that an important consumer can’t deal with it.
> > 
> > BTW, there are 3 other places which look the same and can also be converted
to
> > range loops. I believe the conversion should include those and be commit on
> its
> > own.
> 
> Feel free to fix them all in one go.

Sorry, this is not what I want - I want to just fix Clang compilation issue, and
not to bring C++11 change that might require reverting and refixing. And also
retesting - changing < to != in that for loop is a noop change that doesn't
affect generated code and cannot fail on platforms that I don't have access to.
While using range for loop potentially opens a can of worms.

[Lei Zhang, 2016-01-06 23:41:45]

On 2016/01/06 23:33:14, Dmitry Skiba wrote:
> On 2016/01/06 22:32:25, Lei Zhang wrote:
> > On 2016/01/06 22:13:49, Dmitry Skiba wrote:
> > > On 2016/01/06 22:08:16, Mark Mentovai wrote:
> > > > The easiest way to know if we can use range-based for is to try it and
go
> > back
> > > > if it turns out that an important consumer can’t deal with it.
> > > 
> > > BTW, there are 3 other places which look the same and can also be
converted
> to
> > > range loops. I believe the conversion should include those and be commit
on
> > its
> > > own.
> > 
> > Feel free to fix them all in one go.
> 
> Sorry, this is not what I want - I want to just fix Clang compilation issue,
and
> not to bring C++11 change that might require reverting and refixing. And also
> retesting - changing < to != in that for loop is a noop change that doesn't
> affect generated code and cannot fail on platforms that I don't have access
to.
> While using range for loop potentially opens a can of worms.

That's fine. We can land this and I'll do the C++11 change separately. lgtm

Let me know if you need me to land this for you.

[Dmitry Skiba, 2016-01-06 23:58:24]

On 2016/01/06 23:41:45, Lei Zhang wrote:
> On 2016/01/06 23:33:14, Dmitry Skiba wrote:
> > On 2016/01/06 22:32:25, Lei Zhang wrote:
> > > On 2016/01/06 22:13:49, Dmitry Skiba wrote:
> > > > On 2016/01/06 22:08:16, Mark Mentovai wrote:
> > > > > The easiest way to know if we can use range-based for is to try it and
> go
> > > back
> > > > > if it turns out that an important consumer can’t deal with it.
> > > > 
> > > > BTW, there are 3 other places which look the same and can also be
> converted
> > to
> > > > range loops. I believe the conversion should include those and be commit
> on
> > > its
> > > > own.
> > > 
> > > Feel free to fix them all in one go.
> > 
> > Sorry, this is not what I want - I want to just fix Clang compilation issue,
> and
> > not to bring C++11 change that might require reverting and refixing. And
also
> > retesting - changing < to != in that for loop is a noop change that doesn't
> > affect generated code and cannot fail on platforms that I don't have access
> to.
> > While using range for loop potentially opens a can of worms.
> 
> That's fine. We can land this and I'll do the C++11 change separately. lgtm
> 
> Let me know if you need me to land this for you.

Thanks! Yes, I'm having issues with 'gi cl land' - is that what I should use?
"
error: failed to push some refs to
'https://chromium.googlesource.com/breakpad/breakpad/src.git'
Failed to push. If this persists, please file a bug.
"

[Lei Zhang, 2016-01-07 00:14:50]

On 2016/01/06 23:58:24, Dmitry Skiba wrote:
> On 2016/01/06 23:41:45, Lei Zhang wrote:
> > On 2016/01/06 23:33:14, Dmitry Skiba wrote:
> > > On 2016/01/06 22:32:25, Lei Zhang wrote:
> > > > On 2016/01/06 22:13:49, Dmitry Skiba wrote:
> > > > > On 2016/01/06 22:08:16, Mark Mentovai wrote:
> > > > > > The easiest way to know if we can use range-based for is to try it
and
> > go
> > > > back
> > > > > > if it turns out that an important consumer can’t deal with it.
> > > > > 
> > > > > BTW, there are 3 other places which look the same and can also be
> > converted
> > > to
> > > > > range loops. I believe the conversion should include those and be
commit
> > on
> > > > its
> > > > > own.
> > > > 
> > > > Feel free to fix them all in one go.
> > > 
> > > Sorry, this is not what I want - I want to just fix Clang compilation
issue,
> > and
> > > not to bring C++11 change that might require reverting and refixing. And
> also
> > > retesting - changing < to != in that for loop is a noop change that
doesn't
> > > affect generated code and cannot fail on platforms that I don't have
access
> > to.
> > > While using range for loop potentially opens a can of worms.
> > 
> > That's fine. We can land this and I'll do the C++11 change separately. lgtm
> > 
> > Let me know if you need me to land this for you.
> 
> Thanks! Yes, I'm having issues with 'gi cl land' - is that what I should use?
> "
> error: failed to push some refs to
> 'https://chromium.googlesource.com/breakpad/breakpad/src.git'
> Failed to push. If this persists, please file a bug.
> "

CL for landing: https://codereview.chromium.org/1566893002

You are not in a standalone Breakpad checkout and you don't have committer
anyway.