Fix memtrack_helper for newer versions Android.

Fix memtrack_helper for newer versions Android.

Compatible with lazy initialisation, no memtrack_init() required.
Also fixed buffer overflow.

BUG=672780
Committed: https://crrev.com/8bd5c585ee8066834142fecd91a6e479359ca832
Cr-Commit-Position: refs/heads/master@{#438844}

[kraynov, 2016-12-14 15:37:04]

kraynov@chromium.org changed reviewers:
+ primiano@chromium.org

[kraynov, 2016-12-14 15:37:04]

PTAL

[Primiano Tucci (use gerrit), 2016-12-14 15:51:16]

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
File tools/android/memtrack_helper/memtrack_helper.c (right):

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.c:28: struct memtrack_proc;
why this? we don't care what's the real type of the handle.

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.c:67: buf[rsize] = '\0';
now this will crash if recv fails and rsize = -1
I think what you want here is:

ssize_t rsize = recv(client_sock, buf, sizeof(buf) - 1, 0)
buf[sizeof(buf) - 1] = '\0';

So you are always guaranteed to have at least a NUL at the end

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.c:197: if
(memtrack_proc_get(handle, getpid()) != 0)
Hmm I'd remove this. Since this is a non-(Java)-managed  process I can see this
failing in the future.
Just proc_new should be enough as a smoke test.

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
File tools/android/memtrack_helper/memtrack_helper.h (right):

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.h:32: static inline void log_pid()
{
This is used only in one place, only in the .cc file.
Just add those lines there.
The deal with this file was to share code between the damon and the test client.

[kraynov, 2016-12-15 15:40:12]

Description was changed from

==========
Fix memtrack_helper for newer versions Android.

1. Compatible with lazy initialisation, no memtrack_init() required.
2. Fixed daemonisation.
3. Fixed buffer overflow.

BUG=672780
==========

to

==========
Fix memtrack_helper for newer versions Android.

Compatible with lazy initialisation, no memtrack_init() required.
Also fixed buffer overflow.

BUG=672780
==========

[kraynov, 2016-12-15 15:43:48]

PTAL

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
File tools/android/memtrack_helper/memtrack_helper.c (right):

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.c:28: struct memtrack_proc;
On 2016/12/14 15:51:16, Primiano Tucci wrote:
> why this? we don't care what's the real type of the handle.

Done.

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.c:67: buf[rsize] = '\0';
On 2016/12/14 15:51:16, Primiano Tucci wrote:
> now this will crash if recv fails and rsize = -1
> I think what you want here is:
> 
> ssize_t rsize = recv(client_sock, buf, sizeof(buf) - 1, 0)
> buf[sizeof(buf) - 1] = '\0';
> 
> So you are always guaranteed to have at least a NUL at the end

Done.

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.c:197: if
(memtrack_proc_get(handle, getpid()) != 0)
On 2016/12/14 15:51:16, Primiano Tucci wrote:
> Hmm I'd remove this. Since this is a non-(Java)-managed  process I can see
this
> failing in the future.
> Just proc_new should be enough as a smoke test.

Acknowledged.

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
File tools/android/memtrack_helper/memtrack_helper.h (right):

https://codereview.chromium.org/2576713002/diff/1/tools/android/memtrack_help...
tools/android/memtrack_helper/memtrack_helper.h:32: static inline void log_pid()
{
On 2016/12/14 15:51:16, Primiano Tucci wrote:
> This is used only in one place, only in the .cc file.
> Just add those lines there.
> The deal with this file was to share code between the damon and the test
client.

Done.

[Primiano Tucci (use gerrit), 2016-12-15 15:47:16]

LGTM thanks. 28 lines, that's better for a fix :)
Thanks a lot for the patience. I know that it took lot of study to figure out
those lines.

[kraynov, 2016-12-15 15:49:30]

The CQ bit was checked by kraynov@chromium.org

[commit-bot: I haz the power, 2016-12-15 15:49:58]

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, 2016-12-15 15:57:35]

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

[commit-bot: I haz the power, 2016-12-15 15:57:35]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[kraynov, 2016-12-15 16:08:26]

kraynov@chromium.org changed reviewers:
+ jbudorick@chromium.org

[kraynov, 2016-12-15 16:08:27]

+jbudorick

[jbudorick, 2016-12-15 16:14:39]

On 2016/12/15 15:47:16, Primiano Tucci wrote:
> LGTM thanks. 28 lines, that's better for a fix :)
> Thanks a lot for the patience. I know that it took lot of study to figure out
> those lines.

rs lgtm

[kraynov, 2016-12-15 16:15:04]

The CQ bit was checked by kraynov@chromium.org

[commit-bot: I haz the power, 2016-12-15 16:15: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, 2016-12-15 16:19:42]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481818504633820,
"parent_rev": "e55b46f3ed0e3e88d8c86ea10b43cd4e33b3b520", "commit_rev":
"2049943cd871f6f399fc35fed84ca7f54a106f7c"}

[commit-bot: I haz the power, 2016-12-15 16:20:13]

Description was changed from

==========
Fix memtrack_helper for newer versions Android.

Compatible with lazy initialisation, no memtrack_init() required.
Also fixed buffer overflow.

BUG=672780
==========

to

==========
Fix memtrack_helper for newer versions Android.

Compatible with lazy initialisation, no memtrack_init() required.
Also fixed buffer overflow.

BUG=672780

Review-Url: https://codereview.chromium.org/2576713002
==========

[commit-bot: I haz the power, 2016-12-15 16:20:14]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-15 16:21:48]

Description was changed from

==========
Fix memtrack_helper for newer versions Android.

Compatible with lazy initialisation, no memtrack_init() required.
Also fixed buffer overflow.

BUG=672780

Review-Url: https://codereview.chromium.org/2576713002
==========

to

==========
Fix memtrack_helper for newer versions Android.

Compatible with lazy initialisation, no memtrack_init() required.
Also fixed buffer overflow.

BUG=672780

Committed: https://crrev.com/8bd5c585ee8066834142fecd91a6e479359ca832
Cr-Commit-Position: refs/heads/master@{#438844}
==========

[commit-bot: I haz the power, 2016-12-15 16:21:49]

Patchset 3 (id:??) landed as
https://crrev.com/8bd5c585ee8066834142fecd91a6e479359ca832
Cr-Commit-Position: refs/heads/master@{#438844}