chromium/src
0
Fork 0
Code Activity

Fix and warn on use of "%#0" format specifiers.

Browse Source 
%x means "print number as hex". Adding "#" means "prepend 0x"; adding
"0n" (for some decimal n) means "zero-pad to n characters". So it
seems on its face like, say, "%#08x" would be a good way of saying
"print a 32-bit hex value".

There are two problems. The first is that the prefix length is
counted when looking at how much padding is needed, so we would
actually want "%#010x" in this case. But the second, which is worse,
is that for no obvious reason "#" does not prepend "0x" if the value
is zero. So "%#010x" with 0 prints "0000000000" instead of the
desired "0x00000000".

This inconsistency means that "%#" should never be used with zero-
padding (really, with any padding, but there's little incentive to
use it with anything else). Instead, the more obvious "0x%0nx" should
be used.

Bug: none
Change-Id: Iabdc5ca3759ea37a16948c83ad37c729aa49e4c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4917792
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1206438}
This commit is contained in:
Peter Kasting
2023-10-06 15:42:39 +00:00
committed by Chromium LUCI CQ
parent 4d510ec2a5
commit 7c0d98a9b0
7 changed files with 32 additions and 22 deletions

10
PRESUBMIT.py

@ -487,6 +487,16 @@ _BANNED_IOS_EGTEST_FUNCTIONS : Sequence[BanRule] = (
)
_BANNED_CPP_FUNCTIONS : Sequence[BanRule] = (
BanRule(
'%#0',
(
'Zero-padded values that use "#" to add prefixes don\'t exhibit ',
'consistent behavior, since the prefix is not prepended for zero ',
'values. Use "0x%0..." instead.',
),
False,
[_THIRD_PARTY_EXCEPT_BLINK], # Don't warn in third_party folders.
),
BanRule(
r'/\busing namespace ',
(

4
chrome/browser/ash/system_logs/connected_input_devices_log_source.cc

@ -46,9 +46,9 @@ void ConnectedInputDevicesLogSource::ProcessDeviceFillResponse(
if (it != vendor_map.end())
vendor_name.assign(static_cast<std::string>(it->second));
else
vendor_name = base::StringPrintf("%#06x", dev.vendor_id);
vendor_name = base::StringPrintf("0x%04x", dev.vendor_id);
response->emplace(vendor_key, vendor_name);
response->emplace(pid_key, base::StringPrintf("%#06x", dev.product_id));
response->emplace(pid_key, base::StringPrintf("0x%04x", dev.product_id));
}
void ConnectedInputDevicesLogSource::Fetch(SysLogsSourceCallback callback) {

20
chrome/browser/ash/system_logs/connected_input_devices_log_source_unittest.cc

@ -90,7 +90,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchpad_single) {
EXPECT_EQ(vendor_name, it->second);
it = response().find("TOUCHPAD_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -125,7 +125,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchpad_single_other_ext) {
EXPECT_EQ(t1_vendor_name, it->second);
it = response().find("TOUCHPAD_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", t1_pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", t1_pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -162,7 +162,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchpad_single_ts_ext) {
EXPECT_EQ(tp_vendor_name, it->second);
it = response().find("TOUCHPAD_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", tp_pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", tp_pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -193,7 +193,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchscreen_single) {
EXPECT_EQ(vendor_name, it->second);
it = response().find("TOUCHSCREEN_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -233,7 +233,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchscreen_single_other_ext) {
EXPECT_EQ(t1_vendor_name, it->second);
it = response().find("TOUCHSCREEN_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", t1_pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", t1_pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -270,7 +270,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchscreen_single_tp_ext) {
EXPECT_EQ(ts_vendor_name, it->second);
it = response().find("TOUCHSCREEN_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", ts_pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", ts_pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -308,13 +308,13 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchpad_single_touchscreen_single) {
EXPECT_EQ(tp_vendor_name, it->second);
it = response().find("TOUCHPAD_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", tp_pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", tp_pid), it->second);
it = response().find("TOUCHSCREEN_VENDOR");
ASSERT_NE(it, response().end());
EXPECT_EQ(ts_vendor_name, it->second);
it = response().find("TOUCHSCREEN_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", ts_pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", ts_pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -341,7 +341,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchpad_unknown_vendor_single) {
EXPECT_EQ(unknown_vendor_name, it->second);
it = response().find("TOUCHPAD_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());
@ -370,7 +370,7 @@ TEST_F(ConnectedInputDevicesLogSourceTest, Touchscreen_unknown_vendor_single) {
EXPECT_EQ(unknown_vendor_name, it->second);
it = response().find("TOUCHSCREEN_PID");
ASSERT_NE(it, response().end());
EXPECT_EQ(base::StringPrintf("%#06x", pid), it->second);
EXPECT_EQ(base::StringPrintf("0x%04x", pid), it->second);
/* Verify fetch_callback() has not been called again. */
EXPECT_EQ(1U, num_callback_calls());

8
chrome/browser/ash/system_logs/virtual_keyboard_log_source.cc

@ -49,10 +49,10 @@ void VirtualKeyboardLogSource::Fetch(SysLogsSourceCallback callback) {
base::NumberToString(touchscreen_count);
log_data += base::StrCat({"Touchscreen ", touchscreen_count_converted,
" Product ID"}) +
": " + base::StringPrintf("%#06x", device.product_id) + "\n";
": " + base::StringPrintf("0x%04x", device.product_id) + "\n";
log_data += base::StrCat({"Touchscreen ", touchscreen_count_converted,
" Vendor ID"}) +
": " + base::StringPrintf("%#06x", device.vendor_id) + "\n";
": " + base::StringPrintf("0x%04x", device.vendor_id) + "\n";
++touchscreen_count;
}
@ -75,11 +75,11 @@ void VirtualKeyboardLogSource::Fetch(SysLogsSourceCallback callback) {
log_data +=
base::StrCat({"External Keyboard ", external_keyboard_count_converted,
" Product ID"}) +
": " + base::StringPrintf("%#06x", device.product_id) + "\n";
": " + base::StringPrintf("0x%04x", device.product_id) + "\n";
log_data +=
base::StrCat({"External Keyboard ", external_keyboard_count_converted,
" Vendor ID"}) +
": " + base::StringPrintf("%#06x", device.vendor_id) + "\n";
": " + base::StringPrintf("0x%04x", device.vendor_id) + "\n";
++external_keyboard_count;
}

2
net/disk_cache/simple/simple_net_log_parameters.cc

@ -22,7 +22,7 @@ base::Value::Dict NetLogSimpleEntryConstructionParams(
const disk_cache::SimpleEntryImpl* entry) {
base::Value::Dict dict;
dict.Set("entry_hash",
base::StringPrintf("%#016" PRIx64, entry->entry_hash()));
base::StringPrintf("0x%016" PRIx64, entry->entry_hash()));
return dict;
}

6
sandbox/win/src/restricted_token_test.cc

@ -48,7 +48,7 @@ int RunOpenProcessTest(bool unsandboxed,
runner2.SetUnsandboxed(unsandboxed);
return runner2.RunTest(
base::ASCIIToWide(
base::StringPrintf("RestrictedTokenTest_openprocess %lu %#08lX",
base::StringPrintf("RestrictedTokenTest_openprocess %lu 0X%08lX",
runner.process_id(), access_mask))
.c_str());
}
@ -82,7 +82,7 @@ int RunRestrictedOpenProcessTest(bool unsandboxed,
runner2.SetUnsandboxed(unsandboxed);
return runner2.RunTest(
base::ASCIIToWide(
base::StringPrintf("RestrictedTokenTest_openprocess %lu %#08lX",
base::StringPrintf("RestrictedTokenTest_openprocess %lu 0X%08lX",
runner.process_id(), access_mask))
.c_str());
}
@ -103,7 +103,7 @@ int RunRestrictedSelfOpenProcessTest(bool add_random_sid, DWORD access_mask) {
return runner.RunTest(
base::ASCIIToWide(
base::StringPrintf("RestrictedTokenTest_currentprocess_dup %#08lX",
base::StringPrintf("RestrictedTokenTest_currentprocess_dup 0X%08lX",
access_mask))
.c_str());
}

4
ui/events/ozone/evdev/keyboard_imposter_checker_evdev.cc

@ -64,10 +64,10 @@ bool KeyboardImposterCheckerEvdev::FlagIfImposter(
converter->SetSuspectedImposter(true);
LOG(WARNING) << "Device Name: " << converter->input_device().name
<< " Vendor ID: "
<< base::StringPrintf("%#06x",
<< base::StringPrintf("0x%04x",
converter->input_device().vendor_id)
<< " Product ID: "
<< base::StringPrintf("%#06x",
<< base::StringPrintf("0x%04x",
converter->input_device().product_id)
<< " has been flagged as a suspected imposter keyboard";
#if BUILDFLAG(IS_CHROMEOS_ASH)