New issue
Advanced search Search tips

Issue 684800 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Remove direct dependency on glib from debugd code

Project Member Reported by benchan@chromium.org, Jan 24 2017

Issue description

debugd currently has a direct dependency on glib due to its use of g_base64_encode() from glib for base64 encoding (introduced in https://chromium-review.googlesource.com/c/35531). Given that libchrome now provides base::Base64Encode() for base64 encoding, we should migrate the debugd code to use base::Base64Encode() and remove the direct dependency on glib.
 
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/a8d97a65f81898bd8875b0db88ffb000a7b173a5

commit a8d97a65f81898bd8875b0db88ffb000a7b173a5
Author: Ben Chan <benchan@chromium.org>
Date: Tue Jan 24 21:58:02 2017

chromeos-base/debugd: remove explicit dependency on dev-libs/glib

CL:432036 removes the direct use of glib code in debugd. This CL removes
the explicit dependency on dev-libs/glib from the debugd ebuild. debugd
still indirectly depends on glib via the dependency on
dev-libs/dbus-c++.

BUG= chromium:684800 
CQ-DEPEND=CL:432036
TEST=`FEATURES=test emerge-$BOARD chromeos-base/debugd`

Change-Id: If61d52cfe5eadab6b1e4183dd09c355e647d9d79
Reviewed-on: https://chromium-review.googlesource.com/432003
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a8d97a65f81898bd8875b0db88ffb000a7b173a5/chromeos-base/debugd/debugd-9999.ebuild

Project Member

Comment 3 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/ab93abf5d4d4c33f733dafe8d46350f25bf5f476

commit ab93abf5d4d4c33f733dafe8d46350f25bf5f476
Author: Ben Chan <benchan@chromium.org>
Date: Tue Jan 24 21:32:51 2017

debugd: use base::Base64Encode for Base64 encoding

CL:35531 modified the LogTool helper to encode any non-UTF8 log content
in Base64 before returning the content over DBus as a DBus string. Back
then, libchrome didn't provide any Base64 encoding function, so we used
g_base64_encode() provided by glib, which is the only dependency on glib
in debugd. Given that libchrome now provides base::Base64Encode(), this
CL migrates LogTool to use base::Base64Encode() instead and removes the
explicit dependency on glib. debugd still indirectly depends on glib via
the dependency on dbus-c++.

BUG= chromium:684800 
TEST=Tested the following from a root shell:
- After a normal boot, run the command below and verify that the output
  includes the content of /var/log/messages in plaintext:

  dbus-send --print-reply --system --dest=org.chromium.debugd \
      /org/chromium/debugd org.chromium.debugd.GetLog string:syslog

- Introduce a non-UTF8 character to /var/log/messages by running
  `printf '\x80' | logger`. Re-run the dbus-send command above and
  verify that the output is encoded in Base64 format. Decode the
  Base64-encoded output and verify that the decoded value matches the
  context of /var/log/messages.

Change-Id: Icfe690858b60863ba2ad8b8194f1384a5d55d1ef
Reviewed-on: https://chromium-review.googlesource.com/432036
Commit-Ready: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/ab93abf5d4d4c33f733dafe8d46350f25bf5f476/debugd/debugd.gyp
[modify] https://crrev.com/ab93abf5d4d4c33f733dafe8d46350f25bf5f476/debugd/src/log_tool.cc

Cc: vapier@chromium.org
Status: Fixed (was: Started)

Comment 5 by dchan@google.com, Apr 17 2017

Labels: VerifyIn-59

Comment 6 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 7 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 8 by dchan@chromium.org, Oct 14 2017

Status: Archived (was: Fixed)

Sign in to add a comment