Skip to content

Commit 5008c4b

Browse files
authored
Fix -d device selection silently matching nothing for bare hex USB IDs (#571)
parse_two_ids() read a token without a 0x prefix as decimal, and std::from_chars reports success when it consumes only part of the input. "1b1c" therefore stopped at the 'b' and yielded 1, so -d 1b1c:0a64 built the filter 0001:0000, matched nothing, and surfaced as "No supported device found" - as though the device were unplugged. The help text has documented this form (1038:12ad) all along, and 12ad cannot be anything but hex. parse_two_ids() gains a base for unprefixed tokens, defaulting to 10 so existing callers are unaffected. The two call sites parsing USB vendor/product IDs - -d in main.cpp and --device in dev.cpp - ask for base 16, matching how lsusb and our own device listing print them. An explicit 0x prefix still forces hex. --usage in dev mode is not a USB ID and documents a decimal range, so it keeps base 10. Partial parses are now rejected outright rather than truncated, so a malformed value reports a format error instead of quietly becoming a filter that matches nothing. -d also cast the parsed IDs straight to uint16_t. An out-of-range value wrapped to 0, and matchesDevice() treats 0 as "no filter", so -d 10000:0a64 matched any vendor and -d 10000:10000 matched everything connected. Both are now rejected; 0 itself stays accepted as the existing no-filter default. Note: -d 1234:5678 previously meant decimal (1234, 5678) and now means (0x1234, 0x5678).
1 parent a279bb8 commit 5008c4b

5 files changed

Lines changed: 66 additions & 17 deletions

File tree

‎cli/dev.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -212,10 +212,10 @@ struct DevOptions {
212212
for (int c; (c = getopt_long(argc, argv, "d:i:lu:s:m:f:rg:t:hR:", long_opts.data(), &option_index)) != -1;) {
213213
switch (c) {
214214
case 'd': {
215-
auto ids = headsetcontrol::parse_two_ids(optarg);
215+
auto ids = headsetcontrol::parse_two_ids(optarg, 16);
216216
if (!ids || !in_range(ids->first, 1, 65535) || !in_range(ids->second, 1, 65535)) {
217-
std::cerr << "Invalid --device. Use format: VENDORID:PRODUCTID (1-65535 or 0x1-0xffff)\n"
218-
<< " Example: --device 0x1b1c:0x1b27\n";
217+
std::cerr << "Invalid --device. Use format: VENDORID:PRODUCTID (hex, 1-ffff)\n"
218+
<< " Example: --device 1b1c:1b27\n";
219219
return std::nullopt;
220220
}
221221
opts.vendorid = static_cast<uint16_t>(ids->first);

‎cli/main.cpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,19 @@ std::optional<cli::ParseError> configureParser(cli::ArgumentParser& parser, Opti
191191
.custom('d', "device", cli::ArgRequirement::Required, [&opts](std::optional<std::string_view> arg) -> std::optional<cli::ParseError> {
192192
if (!arg)
193193
return cli::ParseError { "requires vendor:product", "device" };
194-
auto ids = headsetcontrol::parse_two_ids(*arg);
194+
// USB IDs are hex, and are written without a 0x prefix by lsusb
195+
// and by our own device listing.
196+
auto ids = headsetcontrol::parse_two_ids(*arg, 16);
195197
if (!ids) {
196198
return cli::ParseError { "format: vendorid:productid", "device" };
197199
}
200+
// Narrowing an out-of-range ID would wrap it, and a wrapped-to-zero
201+
// ID reads as "no filter" - so -d 10000:10000 would silently match
202+
// every device instead of none.
203+
constexpr int id_max = 0xffff;
204+
if (ids->first < 0 || ids->first > id_max || ids->second < 0 || ids->second > id_max) {
205+
return cli::ParseError { "ids must be between 0 and ffff", "device" };
206+
}
198207
opts.vendor_id = static_cast<uint16_t>(ids->first);
199208
opts.product_id = static_cast<uint16_t>(ids->second);
200209
return std::nullopt; }, "Select device by vendor:product ID")

‎lib/utility.cpp‎

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ std::vector<float> parse_float_data(std::string_view input)
180180
return result;
181181
}
182182

183-
std::optional<std::pair<int, int>> parse_two_ids(std::string_view input)
183+
std::optional<std::pair<int, int>> parse_two_ids(std::string_view input, int default_base)
184184
{
185185
constexpr std::string_view delimiters = " :.,";
186186

@@ -200,19 +200,23 @@ std::optional<std::pair<int, int>> parse_two_ids(std::string_view input)
200200

201201
std::string_view token = input.substr(pos, end - pos);
202202

203-
// Parse the value (supports hex with 0x prefix)
204-
long val = 0;
203+
// An explicit prefix always wins; otherwise the caller decides how a bare
204+
// token is read, because a USB ID means something different to a decimal.
205+
int base = default_base;
205206
if (token.starts_with("0x") || token.starts_with("0X")) {
206-
auto [ptr, ec] = std::from_chars(token.data() + 2, token.data() + token.size(), val, 16);
207-
if (ec == std::errc()) {
208-
values.push_back(val);
209-
}
210-
} else {
211-
auto [ptr, ec] = std::from_chars(token.data(), token.data() + token.size(), val, 10);
212-
if (ec == std::errc()) {
213-
values.push_back(val);
214-
}
207+
token.remove_prefix(2);
208+
base = 16;
209+
}
210+
211+
long val = 0;
212+
auto [ptr, ec] = std::from_chars(token.data(), token.data() + token.size(), val, base);
213+
214+
// The whole token has to be a number. Accepting a partial parse is how
215+
// "1b1c" read as decimal turns into 1 and quietly selects nothing.
216+
if (ec != std::errc() || ptr != token.data() + token.size()) {
217+
return std::nullopt;
215218
}
219+
values.push_back(val);
216220

217221
pos = end;
218222
}

‎lib/utility.hpp‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,19 @@ ParametricEqualizerSettings parse_parametric_equalizer_settings(std::string_view
8787
/**
8888
* @brief Parse two IDs from a string like "123:456" or "0x1b1c:0x1b27"
8989
*
90+
* A token carrying an explicit 0x prefix is always read as hexadecimal. Bare
91+
* tokens are read in @p default_base, so callers parsing USB vendor and product
92+
* IDs should pass 16: those are conventionally written unprefixed in hex, which
93+
* is how lsusb and this tool's own device listing print them.
94+
*
95+
* A token that is only partly numeric is rejected rather than truncated - read
96+
* as decimal, "1b1c" would otherwise silently become 1.
97+
*
9098
* @param input string to parse
99+
* @param default_base base for tokens without a 0x prefix
91100
* @return pair of IDs if successful, nullopt otherwise
92101
*/
93-
std::optional<std::pair<int, int>> parse_two_ids(std::string_view input);
102+
std::optional<std::pair<int, int>> parse_two_ids(std::string_view input, int default_base = 10);
94103

95104
/**
96105
* @brief Cross-platform sleep for milliseconds

‎tests/test_utilities.cpp‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,33 @@ void testParseTwoIds()
468468
auto empty = parse_two_ids("");
469469
ASSERT_FALSE(empty.has_value(), "Empty string should fail");
470470

471+
// A token that is only partly numeric must be rejected, not truncated
472+
auto partial = parse_two_ids("1b1c:0a64");
473+
ASSERT_FALSE(partial.has_value(), "Bare hex should not parse as decimal");
474+
475+
// Bare hex, as lsusb and our own device listing print USB IDs
476+
auto bare_hex = parse_two_ids("1b1c:0a64", 16);
477+
ASSERT_TRUE(bare_hex.has_value(), "Should parse bare hex in base 16");
478+
ASSERT_EQ(0x1b1c, bare_hex->first, "First ID should be 0x1b1c");
479+
ASSERT_EQ(0x0a64, bare_hex->second, "Second ID should be 0x0a64");
480+
481+
// An explicit prefix still wins over the requested base
482+
auto prefixed = parse_two_ids("0x1b1c:0x0a64", 16);
483+
ASSERT_TRUE(prefixed.has_value(), "Prefixed hex should parse in base 16");
484+
ASSERT_EQ(0x1b1c, prefixed->first, "First ID should be 0x1b1c");
485+
486+
// Digits that are decimal in base 10 and hex in base 16
487+
auto as_decimal = parse_two_ids("1038:1234");
488+
ASSERT_TRUE(as_decimal.has_value(), "Digits should parse as decimal by default");
489+
ASSERT_EQ(1038, as_decimal->first, "Base 10 by default");
490+
auto as_hex = parse_two_ids("1038:1234", 16);
491+
ASSERT_TRUE(as_hex.has_value(), "Digits should parse as hex when asked");
492+
ASSERT_EQ(0x1038, as_hex->first, "Base 16 when requested");
493+
494+
// Not a number in either base
495+
auto garbage = parse_two_ids("zz:yy", 16);
496+
ASSERT_FALSE(garbage.has_value(), "Non-numeric should fail in base 16 too");
497+
471498
std::cout << " ✓ parse_two_ids works correctly" << std::endl;
472499
}
473500

0 commit comments

Comments
 (0)