Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(153)

Issue 23819024: Don't copy empty GUID in CopyIdentifyingProperties. (Closed)

Created:
7 years, 3 months ago by pneubeck (no reviews)
Modified:
7 years, 3 months ago
Reviewers:
stevenjb
CC:
chromium-reviews, gauravsh+watch_chromium.org, gspencer+watch_chromium.org, stevenjb+watch_chromium.org, oshima+watch_chromium.org
Visibility:
Public.

Description

Don't copy empty GUID in CopyIdentifyingProperties. This function copied an empty GUID leading to Shill not matching the right networks. This change prevents copying an empty GUID and also checks that all other (required) identifying properties are non-empty. BUG=280146 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221715

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use NET_LOG_ERROR. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -10 lines) Patch
M chromeos/network/shill_property_util.cc View 1 3 chunks +27 lines, -10 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
pneubeck (no reviews)
7 years, 3 months ago (2013-09-05 13:51:32 UTC) #1
stevenjb
lgtm https://codereview.chromium.org/23819024/diff/1/chromeos/network/shill_property_util.cc File chromeos/network/shill_property_util.cc (right): https://codereview.chromium.org/23819024/diff/1/chromeos/network/shill_property_util.cc#newcode233 chromeos/network/shill_property_util.cc:233: LOG(ERROR) << "Missing required identifying properties."; Seems worth ...
7 years, 3 months ago (2013-09-05 17:00:40 UTC) #2
pneubeck (no reviews)
https://codereview.chromium.org/23819024/diff/1/chromeos/network/shill_property_util.cc File chromeos/network/shill_property_util.cc (right): https://codereview.chromium.org/23819024/diff/1/chromeos/network/shill_property_util.cc#newcode233 chromeos/network/shill_property_util.cc:233: LOG(ERROR) << "Missing required identifying properties."; On 2013/09/05 17:00:41, ...
7 years, 3 months ago (2013-09-06 12:53:33 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pneubeck@chromium.org/23819024/7001
7 years, 3 months ago (2013-09-06 12:53:43 UTC) #4
commit-bot: I haz the power
7 years, 3 months ago (2013-09-06 16:22:29 UTC) #5
Message was sent while issue was closed.
Change committed as 221715

Powered by Google App Engine
This is Rietveld 408576698