New issue
Advanced search Search tips

Issue 808623 link

Starred by 1 user
Status: Fixed
Owner:
thakis@chromium.org
Closed: Feb 2018
Cc:
satorux@chromium.org
hashimoto@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 177475



Sign in to add a comment

shill_ipconfig_client.cc looks like it might be missing a break statement:

Project Member Reported by thakis@chromium.org, Feb 2 2018 
added here: https://codereview.chromium.org/9875013/diff/28001/chrome/browser/chromeos/dbus/flimflam_ipconfig_client.cc

07   switch (value.GetType()) {
 108     case base::Value::TYPE_LIST: {
 109       const base::ListValue* list_value = NULL;
         ....
 124       variant_writer.CloseContainer(&array_writer);
 125       writer.CloseContainer(&variant_writer);
 126     }
 127     case base::Value::TYPE_BOOLEAN:
 128     case base::Value::TYPE_INTEGER:
 129     case base::Value::TYPE_DOUBLE:
 130     case base::Value::TYPE_STRING:
 131       dbus::AppendBasicTypeValueDataAsVariant(&writer, value);
 132       break;
 133     default:
 134       DLOG(ERROR) << "Unexpected type " << value.GetType();
 135   }


Note that the case doesn't end in "break;", so TYPE_LIST falls through to the bool/int/doub/str case. Is this intentional.
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 5 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f40598b44dfeaea73f66bfb22b36fc3c6360c8a3

commit f40598b44dfeaea73f66bfb22b36fc3c6360c8a3
Author: Nico Weber <thakis@chromium.org>
Date: Mon Feb 05 17:42:53 2018

Fix -Wimplicit-fallthrough warnings for Chrome OS.

Adds a missing break that made AppendBasicTypeValueDataAsVariant()
add an additional extra variant for base::Value::Type::LIST.
The extra empty value is ignored by the ipconfig service, so this
doesn't change behavior; it just reduces memory traffic a tiny bit.

This CL was uploaded by git cl split.

R=hashimoto@chromium.org

Bug:  177475 , 808623 
Change-Id: I570798f6420a1e0e84e651bec67ae92ad97ac808
Reviewed-on: https://chromium-review.googlesource.com/899794
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Nico Weber <thakis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534412}
[modify] https://crrev.com/f40598b44dfeaea73f66bfb22b36fc3c6360c8a3/chromeos/dbus/shill_ipconfig_client.cc

Comment 2 by thakis@chromium.org, Feb 5 2018

Owner: thakis@chromium.org
Status: Fixed (was: Untriaged)
hashimoto explained to me how things are supposed to work on the review; see cl description in comment 1.
►

Sign in to add a comment