New issue
Advanced search Search tips

Issue 791222 link

Starred by 1 user
Status: Fixed
Owner:
yuweih@chromium.org
Closed: Dec 2017
Cc:
ios-bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

[CRD iOS][Code Analysis] Fix error prone codepaths

Project Member Reported by yuweih@chromium.org, Dec 2 2017 
Here is the clang static analysis report for ios_remoting_google_app:
gpaste/6413724365619200

The code analyzer has found a few warnings:
* remoting/client/audio/audio_player_buffer.cc:169: |next_frame| is never read after `next_frame += bytes_to_copy`.
* remoting/ios/keychain_wrapper.mm:136: Potentially calling dictionaryWithDictionary with nil.
* Looks like view controllers should always call [super viewDidLoad].
* Some init methods return nil if something wrong is going on. We might want to handle or DCHECK that.
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 6 2017 

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

commit d01d9fb62af2bca9a08632759f80b6efab61f107
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed Dec 06 22:00:51 2017

[CRD iOS] Fix warnings from clang static analyzer

This CL fixes a few warnings reported by the clang static analyzer:
* Remove the unnecessary `next_frame += bytes_to_copy` line from
  AudioPlayerBuffer.
* Call dictionaryWithDictionary with empty dictionary if data is nil.
* Add [super viewDidLoad] calls to view controllers.

Note that none of them causes actual problem in the official app.

Bug:  791222 
Change-Id: Icaaf8a43481eb68950dbf6c25c9d7789237aed5e
Reviewed-on: https://chromium-review.googlesource.com/809928
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522216}
[modify] https://crrev.com/d01d9fb62af2bca9a08632759f80b6efab61f107/remoting/client/audio/audio_player_buffer.cc
[modify] https://crrev.com/d01d9fb62af2bca9a08632759f80b6efab61f107/remoting/ios/app/app_view_controller_chromium.mm
[modify] https://crrev.com/d01d9fb62af2bca9a08632759f80b6efab61f107/remoting/ios/app/first_launch_view_controller.mm
[modify] https://crrev.com/d01d9fb62af2bca9a08632759f80b6efab61f107/remoting/ios/app/host_fetching_error_view_controller.mm
[modify] https://crrev.com/d01d9fb62af2bca9a08632759f80b6efab61f107/remoting/ios/app/host_fetching_view_controller.mm
[modify] https://crrev.com/d01d9fb62af2bca9a08632759f80b6efab61f107/remoting/ios/keychain_wrapper.mm
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 7 2017 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/01e2242e8300c57b0037b1d7605e17dee9153ce8

commit 01e2242e8300c57b0037b1d7605e17dee9153ce8
Author: Yuwei Huang <yuweih@chromium.org>
Date: Thu Dec 07 01:01:17 2017

Comment 3 by yuweih@chromium.org, Dec 7 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment