Add system setting to mirror bluetooth pairing database to sd card #1787
Conversation
|
Tested working on several firmwares. Please advise what can be done to better match atmosphere's code style/aesthetic :) |
| Result setsysGetBluetoothDevicesSettingsFwd(Service *s, s32 *total_out, SetSysBluetoothDevicesSettings *settings, s32 count) { | ||
| return serviceDispatchOut(s, 12, *total_out, | ||
| .buffer_attrs = { SfBufferAttr_HipcMapAlias | SfBufferAttr_Out }, | ||
| .buffers = { { settings, count*sizeof(SetSysBluetoothDevicesSettings) } }, |
There was a problem hiding this comment.
space between operator and operands.
| @@ -389,6 +389,12 @@ namespace ams::settings::fwdbg { | |||
| /* 0 = Disabled, 1 = Enabled */ | |||
| R_ABORT_UNLESS(ParseSettingsItemValue("atmosphere", "enable_log_manager", "u8!0x0")); | |||
|
|
|||
| /* Controls whether the bluetooth pairing database is redirected to sd card for synchronization between sysnand and/or multiple emummcs. */ | |||
There was a problem hiding this comment.
In comments:
/* Controls whether the bluetooth pairing database is redirected to the SD card (shared across sysmmc/all emummcs) */
/* NOTE: On <13.0.0, the database size was 10 instead of 20; booting pre-13.0.0 will truncate the database. */
| /* Backup local database to sd card. */ | ||
| R_TRY(WriteExternalBluetoothDatabase(out.GetPointer(), out_count.GetValue())); | ||
| } | ||
| else { |
There was a problem hiding this comment.
Move to be on same line as previous }
| } | ||
| else { | ||
| /* Read the external database from sd card. */ | ||
| R_TRY(ReadExternalBluetoothDatabase(out.GetPointer(), out.GetSize(), reinterpret_cast<size_t*>(out_count.GetPointer()))); |
There was a problem hiding this comment.
This is an invalid reinterpret_cast and will corrupt memory by writing 8 bytes to a 32-bit storage.
Also, reinterpret_cast<size_t *> preferred to reinterpret_cast<size_t*>.
Make the function take in the correct pointer size, do not reinterpret_cast pointers of different sizes in this way.
Also, I don't understand why you switched around the parameter order, it should be s32 *, then buffer.
| R_TRY(WriteExternalBluetoothDatabase(settings.GetPointer(), settings.GetSize())); | ||
|
|
||
| /* Also allow the updated database to be stored to system save as usual. */ | ||
| return sm::mitm::ResultShouldForwardToSession(); |
There was a problem hiding this comment.
This makes me uncomfortable.
Please make it a forwarding command so that the function ends in R_TRY(); return ResultSuccess(); ?
| if (hos::GetVersion() < hos::Version_13_0_0) { | ||
| /* Convert to new database format if running on a firmware below 13.0.0 */ | ||
| auto tmp_entry = std::make_unique<SetSysBluetoothDevicesSettings>(); | ||
| for (u32 i = 0; i < total_entries; ++i) { |
There was a problem hiding this comment.
size_t i, because otherwise this can infinite loop.
| @@ -87,6 +88,87 @@ namespace ams::mitm::settings { | |||
| return ResultSuccess(); | |||
| } | |||
|
|
|||
| const char *g_external_bluetooth_db_location = "@Sdcard:/atmosphere/bluetooth_devices.db"; | |||
|
|
|||
| bool ExternalBluetoothDatabaseEnabled(void) { | |||
There was a problem hiding this comment.
IsExternalBluetoothDatabaseEnabled()
Functions should never have explicit void parameter.
| return en; | ||
| } | ||
|
|
||
| bool HasExternalBluetoothDatabase(void) { |
There was a problem hiding this comment.
Functions should never have explicit void parameter.
| @@ -87,6 +88,87 @@ namespace ams::mitm::settings { | |||
| return ResultSuccess(); | |||
| } | |||
|
|
|||
| const char *g_external_bluetooth_db_location = "@Sdcard:/atmosphere/bluetooth_devices.db"; | |||
There was a problem hiding this comment.
If this is a true constant, please put it in an anonymous namespace at top of file (inside the file's normal namespace), and also please mark it constexpr.
constexpr const char ExternalBluetoothDatabasePath[] = "@Sdcard:/atmosphere/bluetooth_devices.db";
There was a problem hiding this comment.
Wanted to ask about this one. Is it fine to write the whole path out as a constant like this? I saw there was ams::fs::impl::SdCardFileSystemMountName for the mount name and abunch of helper functions in amsmitm_fs_utils.hpp
Also, are you fine with storing the db in the /atmosphere root directory or should it go in a subdirectory somewhere?
config_templates/system_settings.ini
Outdated
| @@ -64,6 +64,11 @@ | |||
| ; Note that this setting is ignored (and treated as 1) when htc is enabled. | |||
| ; 0 = Disabled, 1 = Enabled | |||
| ; enable_log_manager = u8!0x0 | |||
| ; Controls whether the bluetooth pairing database is redirected to sd card for synchronization between sysnand and/or multiple emummcs | |||
There was a problem hiding this comment.
As INI comment:
/* Controls whether the bluetooth pairing database is redirected to the SD card (shared across sysmmc/all emummcs) */
/* NOTE: On <13.0.0, the database size was 10 instead of 20; booting pre-13.0.0 will truncate the database. */
ndeadly
force-pushed
the
bluetooth-db-redirect
branch
from
9f62728
to
8b21161
Compare
This adds an opt-in setting to system_settings.ini to enable the console's bluetooth pairing database to be mirrored to a file on the sd card. This facilitates the synchronisation of device pairings between sysnand and/or one or more emummc setups. Paired with Mission Control's bluetooth host address spoofing feature it could even be used to unify device pairings across multiple consoles.
With the release of bluetooth audio support in firmware 13.0.0 the size of the pairing database was doubled from 10 to 20 entries, and the internal layout of fields was changed slightly. We will only store the database in the new format and convert to/from as necessary. Booting a pre-13.0.0 firmware with more than 10 paired devices present in the sd database will cause the database to be truncated to the 10 most recently paired devices.