Extracts more information from serial devices on Windows

Extracts more information from serial devices on Windows

At rockot@'s suggestion, I tested this with a small Chrome app that I
wrote (https://github.com/zeptonaut/serial-device-enumerator-app) that
opens a window showing all information that Chrome knows about serial
devices. We chose this route over unit tests because we didn't think
it was wise to write unit tests that assumed specific devices were
plugged into our test servers.

As far as devices, I used an FTDI Chipset High Speed USB 2.0 to Serial
RS-232 DB-9 Converter. This was useful because it's one of not-so-many
devices that has both serial properties (COM port) and USB properties
(vendor ID, device ID, and display name).

BUG=522217
Committed: https://crrev.com/e940c46e8e7b40367a4c151656550693476ea870
Cr-Commit-Position: refs/heads/master@{#351163}

[charliea (OOO until 10-5), 2015-09-21 20:49:28]

Patchset #2 (id:20001) has been deleted

[charliea (OOO until 10-5), 2015-09-21 20:51:01]

Patchset #2 (id:40001) has been deleted

[charliea (OOO until 10-5), 2015-09-21 22:20:37]

Patchset #1 (id:1) has been deleted

[charliea (OOO until 10-5), 2015-09-21 22:20:50]

charliea@chromium.org changed reviewers:
+ eakuefner@chromium.org

[charliea (OOO until 10-5), 2015-09-22 18:29:02]

charliea@chromium.org changed reviewers:
+ rockot@chromium.org
- eakuefner@chromium.org

[Ken Rockot(use gerrit already), 2015-09-23 17:30:18]

LGTM

https://codereview.chromium.org/1357993002/diff/60001/device/serial/serial_de...
File device/serial/serial_device_enumerator_win.cc (right):

https://codereview.chromium.org/1357993002/diff/60001/device/serial/serial_de...
device/serial/serial_device_enumerator_win.cc:35: (PBYTE)buffer.get(),
buffer_size,
buffer.get() is always nullptr here. Why not just use nullptr? Casting to PBYTE
should then be unnecessary too (and if not, please use static_cast instead of
C-style casts)

[charliea (OOO until 10-5), 2015-09-28 13:22:54]

https://codereview.chromium.org/1357993002/diff/60001/device/serial/serial_de...
File device/serial/serial_device_enumerator_win.cc (right):

https://codereview.chromium.org/1357993002/diff/60001/device/serial/serial_de...
device/serial/serial_device_enumerator_win.cc:35: (PBYTE)buffer.get(),
buffer_size,
On 2015/09/23 17:30:18, Ken Rockot wrote:
> buffer.get() is always nullptr here. Why not just use nullptr? Casting to
PBYTE
> should then be unnecessary too (and if not, please use static_cast instead of
> C-style casts)

Yea, the way that this was written with the while loop, it wasn't the clearest
what was happening. I uploaded a new patch that tries to clarify what I'm doing,
so PTAL.

Essentially, the first time through this loop, buffer is null and we expect to
receive an ERROR_INSUFFICIENT buffer, but the second time through, we pass in a
pointer to a sufficiently large buffer and the call should succeed. Just putting
NULL in the while loop condition would have meant that, in that second iteration
of the loop, we wouldn't have actually passed the buffer to the function, and
thus we wouldn't have pulled out the value we needed to.

Anyhow, all of this should hopefully be clearer in the new revision I uploaded.

[charliea (OOO until 10-5), 2015-09-28 13:42:35]

Patchset #2 (id:80001) has been deleted

[Ken Rockot(use gerrit already), 2015-09-28 15:30:02]

On 2015/09/28 13:22:54, Charlie Andrews wrote:
>
https://codereview.chromium.org/1357993002/diff/60001/device/serial/serial_de...
> File device/serial/serial_device_enumerator_win.cc (right):
> 
>
https://codereview.chromium.org/1357993002/diff/60001/device/serial/serial_de...
> device/serial/serial_device_enumerator_win.cc:35: (PBYTE)buffer.get(),
> buffer_size,
> On 2015/09/23 17:30:18, Ken Rockot wrote:
> > buffer.get() is always nullptr here. Why not just use nullptr? Casting to
> PBYTE
> > should then be unnecessary too (and if not, please use static_cast instead
of
> > C-style casts)
> 
> Yea, the way that this was written with the while loop, it wasn't the clearest
> what was happening. I uploaded a new patch that tries to clarify what I'm
doing,
> so PTAL.
> 
> Essentially, the first time through this loop, buffer is null and we expect to
> receive an ERROR_INSUFFICIENT buffer, but the second time through, we pass in
a
> pointer to a sufficiently large buffer and the call should succeed. Just
putting
> NULL in the while loop condition would have meant that, in that second
iteration
> of the loop, we wouldn't have actually passed the buffer to the function, and
> thus we wouldn't have pulled out the value we needed to.
> 
> Anyhow, all of this should hopefully be clearer in the new revision I
uploaded.

Ah right. That's clearer. Please use nullptr instead of NULL for new code
though. And still LGTM!

[charliea (OOO until 10-5), 2015-09-28 15:40:13]

Patchset #3 (id:120001) has been deleted

[charliea (OOO until 10-5), 2015-09-28 16:43:00]

charliea@chromium.org changed reviewers:
+ tfarina@chromium.org

[charliea (OOO until 10-5), 2015-09-28 16:43:00]

tfarina@, could you give approval for the dependency on third_party/re2?

[tfarina, 2015-09-28 17:15:21]

LGTM

[tfarina, 2015-09-28 17:16:58]

Could you also fix the BUG= line? It says 5222217, bu that does not appear to be
a valid crbug.com number. Is this an internal /b/?

[Ken Rockot(use gerrit already), 2015-09-28 17:22:16]

On 2015/09/28 17:16:58, tfarina wrote:
> Could you also fix the BUG= line? It says 5222217, bu that does not appear to
be
> a valid http://crbug.com number. Is this an internal /b/?

looks like an extra 2 :P

[charliea (OOO until 10-5), 2015-09-28 19:12:32]

On 2015/09/28 17:22:16, Ken Rockot wrote:
> On 2015/09/28 17:16:58, tfarina wrote:
> > Could you also fix the BUG= line? It says 5222217, bu that does not appear
to
> be
> > a valid http://crbug.com number. Is this an internal /b/?
> 
> looks like an extra 2 :P

Sorry about that. Sometimes when I get started typing a number, I just keep
going...

[charliea (OOO until 10-5), 2015-09-28 19:13:27]

The CQ bit was checked by charliea@chromium.org

[charliea (OOO until 10-5), 2015-09-28 19:13:28]

The patchset sent to the CQ was uploaded after l-g-t-m from rockot@chromium.org
Link to the patchset: https://codereview.chromium.org/1357993002/#ps160001
(title: "Rearranged imports to match style guide.")

[commit-bot: I haz the power, 2015-09-28 19:14:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1357993002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1357993002/160001

[commit-bot: I haz the power, 2015-09-28 20:15:26]

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

[commit-bot: I haz the power, 2015-09-28 20:15:27]

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

[charliea (OOO until 10-5), 2015-09-28 20:56:59]

The CQ bit was checked by charliea@chromium.org

[commit-bot: I haz the power, 2015-09-28 20:58:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1357993002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1357993002/160001

[commit-bot: I haz the power, 2015-09-28 21:21:58]

Committed patchset #4 (id:160001)

[commit-bot: I haz the power, 2015-09-28 21:29:11]

Patchset 4 (id:??) landed as
https://crrev.com/e940c46e8e7b40367a4c151656550693476ea870
Cr-Commit-Position: refs/heads/master@{#351163}