memory-infra: Avoid dereferencing polling_state_ when it could be null

memory-infra: Avoid dereferencing polling_state_ when it could be null

BUG=706023
Review-Url: https://codereview.chromium.org/2787533002
Cr-Commit-Position: refs/heads/master@{#460736}
Committed: https://chromium.googlesource.com/chromium/src/+/3d723ac3847809a99d6e46da60b297a0eb816c88

[hjd, 2017-03-29 16:16:25]

hjd@chromium.org changed reviewers:
+ primiano@chromium.org, ssid@chromium.org

[hjd, 2017-03-29 16:16:26]

ptal, thanks

[Primiano Tucci (use gerrit), 2017-03-29 16:21:23]

https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_scheduler.cc (right):

https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_scheduler.cc:174: if (!polling_state_)
I think you need both checks, i.e.
if (!polling_state_ || polling_state_->current_ctate...)

[ssid, 2017-03-29 16:41:46]

https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_scheduler.cc (right):

https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_scheduler.cc:174: if (!polling_state_)
On 2017/03/29 16:21:22, Primiano Tucci wrote:
> I think you need both checks, i.e.
> if (!polling_state_ || polling_state_->current_ctate...)

No the fix is correct as it is. As soon as disabled is set to null. I just
checked the won't condition.

[ssid, 2017-03-29 16:46:38]

https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
File base/trace_event/memory_dump_scheduler.cc (right):

https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
base/trace_event/memory_dump_scheduler.cc:174: if (!polling_state_)
On 2017/03/29 16:41:45, ssid wrote:
> On 2017/03/29 16:21:22, Primiano Tucci wrote:
> > I think you need both checks, i.e.
> > if (!polling_state_ || polling_state_->current_ctate...)
> 
> No the fix is correct as it is. As soon as disabled is set to null. I just
> checked the won't condition.

Maybe a dcheck(enabled)

[chromium-reviews, 2017-03-29 18:40:48]

Not sure, I copy pasted from ssid's CL.

On Wed, Mar 29, 2017 at 5:21 PM <primiano@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
> File base/trace_event/memory_dump_scheduler.cc (right):
>
>
>
https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
> base/trace_event/memory_dump_scheduler.cc:174: if (!polling_state_)
> I think you need both checks, i.e.
> if (!polling_state_ || polling_state_->current_ctate...)
>
> https://codereview.chromium.org/2787533002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[hjd, 2017-03-30 10:50:26]

On 2017/03/29 18:40:48, chromium-reviews wrote:
> Not sure, I copy pasted from ssid's CL.
> 
> On Wed, Mar 29, 2017 at 5:21 PM <mailto:primiano@chromium.org> wrote:
> 
> >
> >
> >
>
https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
> > File base/trace_event/memory_dump_scheduler.cc (right):
> >
> >
> >
>
https://codereview.chromium.org/2787533002/diff/1/base/trace_event/memory_dum...
> > base/trace_event/memory_dump_scheduler.cc:174: if (!polling_state_)
> > I think you need both checks, i.e.
> > if (!polling_state_ || polling_state_->current_ctate...)
> >
> > https://codereview.chromium.org/2787533002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Good idea, I have added the DCHECK.

If we're agreed on the diff lets submit!

[Primiano Tucci (use gerrit), 2017-03-30 11:49:10]

lgtm

[Primiano Tucci (use gerrit), 2017-03-30 11:49:13]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2017-03-30 11:49:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 12:54:22]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490874553776140,
"parent_rev": "223fd71f458346b7db6193ceb6fdaf849c671e4d", "commit_rev":
"3d723ac3847809a99d6e46da60b297a0eb816c88"}

[commit-bot: I haz the power, 2017-03-30 12:55:07]

Description was changed from

==========
memory-infra: Avoid dereferencing polling_state_ when it could be null

BUG=706023
==========

to

==========
memory-infra: Avoid dereferencing polling_state_ when it could be null

BUG=706023

Review-Url: https://codereview.chromium.org/2787533002
Cr-Commit-Position: refs/heads/master@{#460736}
Committed:
https://chromium.googlesource.com/chromium/src/+/3d723ac3847809a99d6e46da60b2...
==========

[commit-bot: I haz the power, 2017-03-30 12:55:08]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/3d723ac3847809a99d6e46da60b2...