diff --git a/CHANGELOG.md b/CHANGELOG.md index 360043923..7f9cbe6bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Features: * For package managers: ImageMagick dependencies can be removed on macOS, Windows and Android as they are no longer used for image logos. * Windows Terminal supports sixel logos out of the box * Changes to a source image now correctly invalidate its cache. (Logo) - * Added GIF and APNG support to `--kitty` image protocol. Set `logo.animationFrame` to `0` to play animations in compatible kitty terminals; positive or negative values select a still frame. Android animation requires Android 12 (API 31); unsupported terminals show the first frame. (Logo) + * Added GIF and APNG support to `--kitty` image protocol. Set `logo.animationFrame` to `0` to play animations in compatible kitty terminals; positive or negative values select a still frame. Android animation requires Android 12 (API 31); terminals without kitty graphics protocol support report an error and fall back to the built-in logo instead. (Logo) * Added cmake option `-DENABLE_IMAGE_LOGO=` (default `ON`) to disable image logos and reduce binary size. `raw` logos remain available. (Logo) * Improved Linux support: * Added RakuOS rum overlay package counting (`{rum}`) and CRUX package detection (`{crux}`). (Packages) @@ -30,7 +30,7 @@ Features: * Improved camera detection; Android 7 (API 24) or later is required. (Camera) * Battery level and charging state are now available to apps; detailed battery information requires root or ADB. (Battery) * Added display mode, physical size, rotation, and HDR information. App-based detection requires Android 13 (API 33); root or ADB can also detect displays on older versions. (Display) - * Added Wi-Fi interface, connection, standard, and security information. Android 11 (API 30) or later is required. Termux:API and location permission are needed to reveal SSID and BSSID; otherwise, they appear as ``. (Wifi) + * Added Wi-Fi interface, connection, standard, and security information. Android 11 (API 30) or later is required. Termux:API is needed for the Wi-Fi permission, and location permission is needed to reveal SSID and BSSID; without location permission the two names appear as ``. (Wifi) * Added Wallpaper detection, which effectively reports `/data/system/users/0/wallpaper`. (Wallpaper) * Added Media detection, root/ADB only. (Media / Player) * Improved macOS support: diff --git a/CMakeLists.txt b/CMakeLists.txt index ad67e2eb9..2acd937c2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,12 +98,16 @@ cmake_dependent_option(ENABLE_ELF "Enable libelf" ON "LINUX OR ANDROID OR Dragon cmake_dependent_option(ENABLE_THREADS "Enable multithreading" ON "Threads_FOUND" OFF) option(ENABLE_IMAGE_LOGO "Enable image logos (sixel / kitty / iTerm / chafa)" ON) -if(ENABLE_IMAGE_LOGO) - cmake_dependent_option(ENABLE_IMAGEMAGICK7 "Enable imagemagick 7" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR Haiku OR GNU" OFF) - cmake_dependent_option(ENABLE_IMAGEMAGICK6 "Enable imagemagick 6" ON "LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR GNU" OFF) - cmake_dependent_option(ENABLE_SIXEL "Enable sixel logo output via the embedded libsixel encoder (Windows, macOS and Android; replaces ImageMagick's SIXEL coder)" ON "WIN32 OR APPLE OR ANDROID" OFF) - cmake_dependent_option(ENABLE_CHAFA "Enable chafa" ON "ENABLE_IMAGEMAGICK6 OR ENABLE_IMAGEMAGICK7 OR WIN32 OR APPLE OR ANDROID" OFF) -endif() +# The four below are declared outside the guard on purpose. `cmake_dependent_option` is a macro: when +# the condition is false it is never expanded, so nothing rewrites the value already sitting in +# CMakeCache.txt, and `cmake -DENABLE_IMAGE_LOGO=OFF .` in an existing build directory would leave +# ENABLE_SIXEL=ON behind -- sixel and chafa would still be compiled and linked while image.c, the +# only file that uses them, is not. The condition does the disabling instead: it is the fourth +# argument's job, and it keeps the option visible and settable in cmake-gui either way. +cmake_dependent_option(ENABLE_IMAGEMAGICK7 "Enable imagemagick 7" ON "ENABLE_IMAGE_LOGO;LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR Haiku OR GNU" OFF) +cmake_dependent_option(ENABLE_IMAGEMAGICK6 "Enable imagemagick 6" ON "ENABLE_IMAGE_LOGO;LINUX OR FreeBSD OR OpenBSD OR NetBSD OR SunOS OR GNU" OFF) +cmake_dependent_option(ENABLE_SIXEL "Enable sixel logo output via the embedded libsixel encoder (Windows, macOS and Android; replaces ImageMagick's SIXEL coder)" ON "ENABLE_IMAGE_LOGO;WIN32 OR APPLE OR ANDROID" OFF) +cmake_dependent_option(ENABLE_CHAFA "Enable chafa" ON "ENABLE_IMAGE_LOGO;ENABLE_IMAGEMAGICK6 OR ENABLE_IMAGEMAGICK7 OR WIN32 OR APPLE OR ANDROID" OFF) option(ENABLE_ZLIB "Enable zlib" ON) option(ENABLE_SYSTEM_YYJSON "Use system provided (instead of fastfetch embedded) yyjson library" OFF) @@ -1931,37 +1935,29 @@ if(LINUX) endif() elseif(APPLE) target_link_libraries(libfastfetch - # reachable from the default structure + PRIVATE "-framework AVFoundation" + PRIVATE "-framework Accelerate" PRIVATE "-framework Cocoa" + PRIVATE "-framework CoreAudio" + PRIVATE "-framework CoreBluetooth" PRIVATE "-framework CoreFoundation" + PRIVATE "-framework CoreMedia" + PRIVATE "-framework CoreVideo" + PRIVATE "-framework CoreWLAN" + PRIVATE "-framework GameController" + PRIVATE "-framework IOBluetooth" PRIVATE "-framework IOKit" + PRIVATE "-framework ImageIO" PRIVATE "-framework Metal" + PRIVATE "-framework OpenCL" + PRIVATE "-framework OpenGL" + PRIVATE "-framework SystemConfiguration" + PRIVATE "-framework VideoToolbox" - # referenced as ObjC classes, so they cannot be lazy loaded - PRIVATE "-framework AVFoundation" # Camera - PRIVATE "-framework CoreBluetooth" # Bluetooth - PRIVATE "-framework IOBluetooth" # Bluetooth, BluetoothRadio - PRIVATE "-framework CoreWLAN" # Wifi - PRIVATE "-framework GameController" # Gamepad - - # only reachable from non-default modules, and referenced as plain C - PRIVATE "-Wl,-lazy_framework,CoreAudio" # Sound - PRIVATE "-Wl,-lazy_framework,CoreMedia" # Camera - PRIVATE "-Wl,-lazy_framework,CoreVideo" # Display, fallback for older macOS versions - PRIVATE "-Wl,-lazy_framework,OpenGL" # OpenGL - PRIVATE "-Wl,-lazy_framework,OpenCL" # OpenCL - PRIVATE "-Wl,-lazy_framework,SystemConfiguration" # DNS - PRIVATE "-Wl,-lazy_framework,VideoToolbox" # Codec - PRIVATE "-Wl,-lazy_framework,ImageIO" # image logos - PRIVATE "-Wl,-lazy_framework,Accelerate" # image logos - - # private frameworks, and only reachable from non-default modules. - # `-weak_framework` already keeps a missing framework from aborting the - # launch, which lazy loading on its own would not do. - PRIVATE "-weak_framework CoreDisplay" # Brightness + PRIVATE "-weak_framework CoreDisplay" PRIVATE "-F /System/Library/PrivateFrameworks" - PRIVATE "-weak_framework DisplayServices" # Brightness - PRIVATE "-weak_framework MediaRemote" # Media + PRIVATE "-weak_framework DisplayServices" + PRIVATE "-weak_framework MediaRemote" ) elseif(WIN32) target_link_libraries(libfastfetch @@ -1978,8 +1974,16 @@ elseif(WIN32) PRIVATE "winbrand" PRIVATE "secur32" PRIVATE "shlwapi" - PRIVATE "windowscodecs" ) + if(ENABLE_IMAGE_LOGO) + # Only src/logo/image/wic.cpp uses WIC, and that file is only compiled with image logos + # enabled (see the WIN32 source list above). Verified by building with + # `-DENABLE_IMAGE_LOGO=OFF` and removing the library from the link line: it links and runs + # without it, and nothing else in the tree references IWIC*. + target_link_libraries(libfastfetch + PRIVATE "windowscodecs" + ) + endif() if(NOT ENABLE_WIN81_COMPAT) target_link_libraries(libfastfetch PRIVATE "mincore" @@ -2060,12 +2064,23 @@ elseif(ANDROID) endif() endif() - # Three APIs (AImageDecoder, AndroidBitmap_compress, AMediaCodec) are newer than the API level this - # build targets -- the first two are 30, and AMediaCodec_getName / AMediaCodec_releaseName are 28 -- - # while fastfetch keeps running on devices below those levels. The NDK headers mark the entry points - # unavailable, so they have to be weak references: otherwise the executable would fail to load on an - # older device. Guarding every use with __builtin_available() is what makes that safe, and turning - # the guard diagnostic into an error is what keeps a use from slipping past it. + # Three APIs are newer than the API level this build targets: AImageDecoder is 30, + # AndroidBitmap_compress is 30, and AMediaCodec_getName / AMediaCodec_releaseName are 28. + # fastfetch keeps running on devices below those levels, and the NDK headers mark the entry + # points unavailable there, so they have to be weak references -- a strong reference would fail + # the load on an older device. Guarding every use with __builtin_available() is what makes that + # safe, and turning the guard diagnostic into an error keeps a use from slipping past it. + # + # Each one lives in the library that provides it, and that library is linked above: + # AImageDecoder_* and AndroidBitmap_compress are exported by libjnigraphics.so, and + # AMediaCodec_* by libmediandk.so. Checked on a device (Android 16, API 36) with + # `readelf --dyn-syms /system/lib64/libjnigraphics.so` and a dlopen/dlsym probe: every + # AImageDecoder_* entry point resolves there and none of them resolves in libandroid.so, which + # is why libandroid.so is *not* linked -- nothing fastfetch calls comes from it. Keep the + # provider library linked for the reason the NDK requires it: an undefined weak symbol resolves + # to NULL rather than failing the load, and FF_ANDROID_API_AT_LEAST() reads the platform level, + # not the symbol, so it would send the call straight through a NULL pointer on every device new + # enough to pass the guard. # # camera_android.c is deliberately not in this list. The camera2 NDK is a *library* that starts at # API 24, and a weak reference weakens the symbol, never the library: DT_NEEDED would still name diff --git a/doc/json_schema.json b/doc/json_schema.json index 85357a8a0..f65c5dcd9 100644 --- a/doc/json_schema.json +++ b/doc/json_schema.json @@ -1761,7 +1761,7 @@ "type": "boolean", "default": false }, - "devType": { + "showType": { "description": "Which Bluetooth stacks to look for", "oneOf": [ { diff --git a/src/common/android/binder.c b/src/common/android/binder.c index 7de6972b7..41e163524 100644 --- a/src/common/android/binder.c +++ b/src/common/android/binder.c @@ -284,3 +284,26 @@ void ffBinderServiceHandleRelease(FFBinderServiceHandle* service) { ioctl(service->binder->fd, BINDER_WRITE_READ, &release); service->handle = 0; } + +void ffBinderAcquiredRelease(FFBinderAcquired* acquired) { + if (acquired->replied == nullptr || *acquired->replied == false) { + return; // nothing was acquired + } + + // A descriptor is not a reference: the kernel installed it in our own fd table when it delivered + // the reply, and nobody else holds it, so closing it is the whole of giving it back. + for (uint32_t i = 0; i < acquired->fdCount; i++) { + close(acquired->fds[i]); + } + + // The mirror of the acquire the transaction appended to its tail, in the same order it took them. + // No binder to talk through means the handles went with it when it was closed. + if (acquired->binder == nullptr || acquired->binder->fd < 0) { + return; + } + + for (uint32_t i = 0; i < acquired->handleCount; i++) { + FFBinderServiceHandle handle = { .binder = acquired->binder, .handle = acquired->handles[i] }; + ffBinderServiceHandleRelease(&handle); + } +} diff --git a/src/common/android/binder.h b/src/common/android/binder.h index ae8c9d47a..4f917070b 100644 --- a/src/common/android/binder.h +++ b/src/common/android/binder.h @@ -19,8 +19,6 @@ // * A handle arriving in a reply is owned by that reply buffer. It has to be acquired // (BC_ACQUIRE + BC_INCREFS) before BC_FREE_BUFFER; otherwise only a weak reference survives and // every later transaction on it fails, because the kernel looks up a strong reference. -// -// See .workbuddy-ai/android-binder-raw-client.md for the full write-up and the tooling. #include "fastfetch.h" // IWYU pragma: keep @@ -251,3 +249,29 @@ typedef struct FFBinderServiceHandle { // Returns the strong and weak references ffBinderLookupService() acquired on the handle. Usable as a // cleanup attribute; safe on a zeroed or already released FFBinderServiceHandle. [[gnu::nonnull(1)]] void ffBinderServiceHandleRelease(FFBinderServiceHandle* service); + +// --------------------------------------------------------------------------------------------- +// Handles a transaction acquired +// --------------------------------------------------------------------------------------------- + +// Whatever ffBinderTransact() collected for the caller, released together on the way out of a +// function -- the handles it acquired, plus the descriptors it delivered, which are owned rather +// than borrowed and would otherwise stay open for the life of the process. +// +// Unlike FFBinderServiceHandle this cannot be declared before the transaction runs, because which +// handles came back is only known afterwards. `replied` is therefore a separate flag the caller +// raises once ffBinderTransact() has returned: it acquires the handles even when it goes on to fail +// (a reply that did not fit the caller buffer still had its BC_ACQUIRE tail sent), so releasing +// without it would drop a reference that was never taken. +typedef struct FFBinderAcquired { + FFBinder* binder; + const uint32_t* handles; + uint32_t handleCount; + const int32_t* fds; + uint32_t fdCount; + const bool* replied; +} FFBinderAcquired; + +// Usable as a cleanup attribute; safe on a zeroed FFBinderAcquired, and on one whose transaction did +// not reply. +[[gnu::nonnull(1)]] void ffBinderAcquiredRelease(FFBinderAcquired* acquired); diff --git a/src/common/android/dex.c b/src/common/android/dex.c index 3b8cdf36d..e8e68a81c 100644 --- a/src/common/android/dex.c +++ b/src/common/android/dex.c @@ -95,28 +95,60 @@ static uint32_t dexUleb128(const uint8_t** p, const uint8_t* end) { return value; } -// encoded_value: one header byte holding ((size - 1) << 5) | type, then that many payload bytes. A -// `static final int` constant is written as VALUE_INT, whose payload is a sign-extended -// little-endian integer. Reading through 64 bits keeps every shift in range even if the size field -// is nonsense; `end` is what keeps a truncated payload from running off the end of the mapping. -static int32_t dexEncodedInt(const uint8_t** p, const uint8_t* end) { +// The type tag a dex encoded_value carries in the low five bits of its header byte, per the +// `encoded_value` table of the dex format (`VALUE_INT = 0x04`, sign-extended four-byte integer). +// 0x1f is VALUE_BOOLEAN in that same table, which is the trap: naming it VALUE_INT and comparing +// against it rejects every real int and accepts booleans. +#define FF_DEX_VALUE_INT 0x04 + +// encoded_value: one header byte holding ((size - 1) << 5) | type, then that many payload bytes. +// +// The `static_values` array is not a list of ints: `IInterface.DESCRIPTOR` is a `static final +// String` and sits in it, and a real Stub class mixes ints, strings and booleans. So a value whose +// tag is not VALUE_INT is skipped -- its payload is stepped over to keep the walk aligned with the +// field list -- and only `isInt` reports whether the *wanted* field ended up being one. Failing on +// the first non-int instead would abort on `DESCRIPTOR`, before the transaction constants that +// follow it are ever reached. Measured on a device: `IWifiManager$Stub` carries 347 static fields +// whose values are mostly VALUE_INT but include that string, and `getConnectionInfo` is 98 there. +// +// A payload wider than four bytes is not an int either, and the byte count is what keeps a size +// field read out of a corrupt file from shifting past the end. Reading through 64 bits keeps every +// shift in range while counting down; `end` is what keeps a truncated payload from running off the +// end of the mapping. +static bool dexEncodedValue(const uint8_t** p, const uint8_t* end, int32_t* result, bool* isInt) { if (*p >= end) { - return 0; + return false; } const uint8_t header = *(*p)++; + const uint32_t type = (uint32_t) (header & 0x1f); const uint32_t size = (uint32_t) (header >> 5) + 1; + if (size > 8) { + return false; + } + uint64_t value = 0; - for (uint32_t i = 0; i < size && i < 8; ++i) { + for (uint32_t i = 0; i < size; ++i) { if (*p >= end) { - break; + return false; + } + if (i < sizeof(value)) { + value |= (uint64_t) (*(*p)++) << (i * 8); + } else { + (void) *(*p)++; } - value |= (uint64_t) (*(*p)++) << (i * 8); } - const uint32_t bits = size * 8; - if (bits < 64 && (value & (1ull << (bits - 1))) != 0) { - value |= ~((1ull << bits) - 1); + + *isInt = type == FF_DEX_VALUE_INT && size <= sizeof(uint32_t); + if (*isInt) { + // VALUE_INT is sign-extended from its own width, not from 32 bits: a one-byte -1 is written + // as `21 ff`, one byte of 0xff, and has to come back as -1 rather than 255. + const uint32_t bits = size * 8; + if ((value & (1ull << (bits - 1))) != 0) { + value |= ~((1ull << bits) - 1); + } } - return (int32_t) (uint32_t) value; + *result = (int32_t) (uint32_t) value; + return true; } // string_data_item: a uleb128 length in UTF-16 code units, then MUTF-8 bytes and a NUL terminator. @@ -216,13 +248,23 @@ static const char* dexStaticInt(const uint8_t* dex, size_t size, const char* cla } fieldIndex += fieldIndexDiff; (void) dexUleb128(&fields, end); // access_flags - const int32_t value = dexEncodedInt(&values, end); + int32_t value; + bool isInt; + if (!dexEncodedValue(&values, end, &value, &isInt)) { + return "The dex static value list is truncated"; + } if (!dexTableInRange(dex, end, fieldIds, (uint64_t) fieldIndex + 1, FF_DEX_FIELD_ID_SIZE)) { return "The dex field table is out of range"; } const char* name = dexString(dex, end, dexU32(fieldIds + (size_t) fieldIndex * FF_DEX_FIELD_ID_SIZE + FF_DEX_OFF_FIELD_ID_NAME)); if (name != nullptr && strcmp(name, fieldName) == 0) { + if (!isInt) { + // The field exists but is not an int, so its value is not the constant asked + // for. Reported here rather than on the first non-int in the list, because the + // list legitimately holds strings (`DESCRIPTOR`) and booleans. + return "The dex static field is not an int"; + } *result = value; return nullptr; } diff --git a/src/common/impl/io_windows.c b/src/common/impl/io_windows.c index 4f4b637ed..d8fb82960 100644 --- a/src/common/impl/io_windows.c +++ b/src/common/impl/io_windows.c @@ -362,8 +362,18 @@ void listFilesRecursively(uint32_t baseLength, FFstrbuf* folder, uint8_t indenta } ffStrbufAppendC(folder, '*'); - WIN32_FIND_DATAA entry; - HANDLE hFind = FindFirstFileA(folder->chars, &entry); + // The wide API on purpose: every path fastfetch holds is UTF-8, and FindFirstFileA would read + // it in the active code page, so a directory whose name the code page cannot represent -- a + // Chinese or Japanese name under an English or German locale, for one -- answers + // ERROR_PATH_NOT_FOUND and the whole listing disappears. The names it returns are converted + // back to UTF-8 below. + wchar_t wideFolder[PATH_MAX]; + if (!NT_SUCCESS(RtlUTF8ToUnicodeN(wideFolder, sizeof(wideFolder), nullptr, folder->chars, folder->length + 1))) { + ffStrbufTrimRight(folder, '*'); + return; + } + WIN32_FIND_DATAW entry; + HANDLE hFind = FindFirstFileW(wideFolder, &entry); ffStrbufTrimRight(folder, '*'); if (hFind == INVALID_HANDLE_VALUE) { return; @@ -371,14 +381,15 @@ void listFilesRecursively(uint32_t baseLength, FFstrbuf* folder, uint8_t indenta do { if (entry.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { - if (ffStrEquals(entry.cFileName, ".") || ffStrEquals(entry.cFileName, "..")) { + if (entry.cFileName[0] == L'.') { continue; } ffStrbufSubstrBefore(folder, folderLength); - ffStrbufAppendS(folder, entry.cFileName); + ffStrbufAppendWS(folder, entry.cFileName); ffStrbufAppendC(folder, '/'); - listFilesRecursively(baseLength, folder, (uint8_t) (indentation + 1), entry.cFileName, pretty); + FF_STRBUF_AUTO_DESTROY name = ffStrbufCreateWS(entry.cFileName); + listFilesRecursively(baseLength, folder, (uint8_t) (indentation + 1), name.chars, pretty); ffStrbufSubstrBefore(folder, folderLength); continue; } @@ -391,8 +402,9 @@ void listFilesRecursively(uint32_t baseLength, FFstrbuf* folder, uint8_t indenta fputs(folder->chars + baseLength, stdout); } - puts(entry.cFileName); - } while (FindNextFileA(hFind, &entry)); + FF_STRBUF_AUTO_DESTROY name = ffStrbufCreateWS(entry.cFileName); + ffStrbufPutTo(&name, stdout); + } while (FindNextFileW(hFind, &entry)); FindClose(hFind); } diff --git a/src/detection/bluetooth/bluetooth_apple.m b/src/detection/bluetooth/bluetooth_apple.m index 1830efea7..cef3f9a5f 100644 --- a/src/detection/bluetooth/bluetooth_apple.m +++ b/src/detection/bluetooth/bluetooth_apple.m @@ -36,7 +36,7 @@ // --------------------------------------------------------------------------------------------- // // Core Bluetooth is the only public route to a peripheral that speaks Low Energy rather than BR/EDR. -// What it will and will not answer was measured on macOS 27 with `.workbuddy-ai/probes/ble/le_probe` +// What it will and will not answer was measured on macOS 27 with a standalone probe program // (12 paired devices, one of them connected), because none of it follows from the headers: // // * `retrieveConnectedPeripheralsWithServices:@[]` answers with **nothing** -- 0 devices in 0.1 ms. @@ -88,7 +88,6 @@ // fills its peer map (0 entries after two seconds, `retrievePairedPeersWithOptions:` answers nil), // and `CBCentralManager.sharedPairingAgent.retrievePairedPeers` answers with 7 of the 11 devices // as bare `CBPeripheral` objects carrying no address, no battery and no class of device. -// `.workbuddy-ai/probes/ble/cb_only_probe.m` is the probe that measured this. // // * `CBPeripheral.state` is not the connection state the caller means. It answers `Disconnected` for // every peripheral the retrieval above returns, and so does `isConnected`; only the private diff --git a/src/detection/gamepad/gamepad_windows.c b/src/detection/gamepad/gamepad_windows.c index 3c95c2a5e..4d4371f1b 100644 --- a/src/detection/gamepad/gamepad_windows.c +++ b/src/detection/gamepad/gamepad_windows.c @@ -97,23 +97,28 @@ static const char* detectKnownDeviceName(uint32_t vendorId, uint32_t productId) static bool readHidReport(HANDLE hHidFile, uint8_t* buffer, uint32_t length, DWORD timeoutMs, DWORD* nBytes) { OVERLAPPED overlapped = {}; DWORD read; - if (ReadFile(hHidFile, buffer, length, &read, &overlapped)) { - *nBytes = read; - return true; - } - if (GetOverlappedResultEx(hHidFile, &overlapped, &read, timeoutMs, TRUE)) { - *nBytes = read; - return true; + if (!ReadFile(hHidFile, buffer, length, &read, &overlapped)) { + // A synchronous failure -- the device was unplugged, the handle went stale, access was + // denied -- does not set up `overlapped` at all, so it still looks like `STATUS_SUCCESS` + // and asking for the result below would report success with zero bytes read. + if (GetLastError() != ERROR_IO_PENDING) { + return false; + } + + if (!GetOverlappedResultEx(hHidFile, &overlapped, &read, timeoutMs, TRUE)) { + // A timeout leaves the read pending, and the driver writes into `buffer` and `overlapped` when + // it eventually completes -- both of which are gone by then: the buffer belongs to the caller + // and the OVERLAPPED sits on this frame. So the read is cancelled by its own OVERLAPPED rather + // than by handle (CancelIo would take every other read on the same handle with it), and the + // cancellation is waited for before returning. + CancelIoEx(hHidFile, &overlapped); + GetOverlappedResult(hHidFile, &overlapped, &read, TRUE); + return false; + } } - // A timeout leaves the read pending, and the driver writes into `buffer` and `overlapped` when - // it eventually completes -- both of which are gone by then: the buffer belongs to the caller - // and the OVERLAPPED sits on this frame. So the read is cancelled by its own OVERLAPPED rather - // than by handle (CancelIo would take every other read on the same handle with it), and the - // cancellation is waited for before returning. - CancelIoEx(hHidFile, &overlapped); - GetOverlappedResult(hHidFile, &overlapped, &read, TRUE); - return false; + *nBytes = read; + return true; } // The DualShock 4 sends a battery level in its state packet, but only in the extended report it @@ -266,7 +271,8 @@ static uint8_t detectSwitchBattery(HANDLE hHidFile, uint8_t* reportBuffer, uint3 } DWORD nBytes; - if (!readHidReport(hHidFile, reportBuffer, reportLength, (DWORD) remaining, &nBytes)) { + // A report never arrives empty, so zero bytes means the read did not deliver one + if (!readHidReport(hHidFile, reportBuffer, reportLength, (DWORD) remaining, &nBytes) || !nBytes) { break; } @@ -420,12 +426,14 @@ const char* ffDetectGamepad(FFlist* devices /* List of FFGamepadDevice */) { DWORD nBytes; switch (batteryKind) { case FF_GAMEPAD_BATTERY_DS4: - if (readHidReport(hHidFile, reportBuffer, caps.InputReportByteLength, FF_IO_TERM_RESP_WAIT_MS, &nBytes)) { + // `reportBuffer` is uninitialised malloc() memory, so a read that delivered + // nothing must not be parsed + if (readHidReport(hHidFile, reportBuffer, caps.InputReportByteLength, FF_IO_TERM_RESP_WAIT_MS, &nBytes) && nBytes) { device->battery = parseDs4Battery(reportBuffer, caps.InputReportByteLength); } break; case FF_GAMEPAD_BATTERY_DUALSENSE: - if (readHidReport(hHidFile, reportBuffer, caps.InputReportByteLength, FF_IO_TERM_RESP_WAIT_MS, &nBytes)) { + if (readHidReport(hHidFile, reportBuffer, caps.InputReportByteLength, FF_IO_TERM_RESP_WAIT_MS, &nBytes) && nBytes) { device->battery = parseDualSenseBattery(reportBuffer, caps.InputReportByteLength); } break; diff --git a/src/detection/initsystem/initsystem_windows.c b/src/detection/initsystem/initsystem_windows.c index a6a90f3ce..561d02449 100644 --- a/src/detection/initsystem/initsystem_windows.c +++ b/src/detection/initsystem/initsystem_windows.c @@ -25,7 +25,7 @@ const char* ffDetectInitSystem(FFInitSystemResult* result) { if (ffIsSystemBasicProcessInfoAvailable()) { // SYSTEM_BASICPROCESS_INFORMATION entries are much smaller than SYSTEM_PROCESS_INFORMATION ones, // so a modest buffer should be enough to contain all processes - SYSTEM_BASICPROCESS_INFORMATION buffer[1024]; + SYSTEM_BASICPROCESS_INFORMATION buffer[1024] = {}; NTSTATUS status = NtQuerySystemInformation(SystemBasicProcessInformation, buffer, sizeof(buffer), NULL); if (status != STATUS_INFO_LENGTH_MISMATCH && !NT_SUCCESS(status)) { goto fallback; diff --git a/src/detection/media/media_android.c b/src/detection/media/media_android.c index 4537ac890..53596e760 100644 --- a/src/detection/media/media_android.c +++ b/src/detection/media/media_android.c @@ -828,6 +828,16 @@ static const char* androidDetectMediaSession(FFMediaResult* media, bool saveCove // artwork are fetched exactly once, for the winner. uint32_t tokens[FF_BINDER_MAX_HANDLES]; uint32_t tokenCount = 0; + // Raised as soon as ffBinderTransact() has replied, because from that point on it is holding a + // strong reference to every handle it collected and the kernel has installed every descriptor it + // delivered into our own fd table. Any early return past that line has to give them back, so one + // cleanup attribute does it rather than a release call repeated at each exit. + // + // `replied` is what decides: ffBinderTransact() acquires the handles before it can fail -- it + // returns an error when the reply did not fit the caller buffer, and the tail that carries + // BC_ACQUIRE runs regardless -- so releasing without the flag would drop one reference too many. + bool replied = false; + [[gnu::cleanup(ffBinderAcquiredRelease)]] FFBinderAcquired acquired = { .binder = &binder, .replied = &replied }; { uint8_t request[FF_MEDIA_ANDROID_REQUEST_SIZE]; FFBinderParcel parcel = ffBinderParcelCreate(request, sizeof(request)); @@ -837,6 +847,7 @@ static const char* androidDetectMediaSession(FFMediaResult* media, bool saveCove FFBinderReply reply = ffBinderReplyCreate(s_reply, sizeof(s_reply)); error = ffBinderTransact(&binder, service.handle, FF_MEDIA_ANDROID_GET_SESSIONS, 0, &parcel, &reply); + replied = true; if (error != nullptr) { return error; } @@ -864,6 +875,13 @@ static const char* androidDetectMediaSession(FFMediaResult* media, bool saveCove for (uint32_t i = 0; i < tokenCount; i++) { tokens[i] = reply.handles[i]; } + acquired.handles = tokens; + acquired.handleCount = tokenCount; + // Every descriptor the reply delivered is ours to close. The art of a session is read through + // one of them, and reading it happens over the fd number rather than over the file, so closing + // it on the way out does not cut the read short. + acquired.fds = reply.fds; + acquired.fdCount = reply.fdCount; } if (tokenCount == 0) { @@ -889,13 +907,7 @@ static const char* androidDetectMediaSession(FFMediaResult* media, bool saveCove androidReadSession(&binder, tokens[best], media, saveCover); - // The tokens were acquired by ffBinderTransact() and are given back one by one; releasing them is - // what keeps a short-lived fastfetch from holding a session open for the life of the process. - for (uint32_t i = 0; i < tokenCount; i++) { - FFBinderServiceHandle token = { .binder = &binder, .handle = tokens[i] }; - ffBinderServiceHandleRelease(&token); - } - + // The tokens were acquired by ffBinderTransact() and are given back by the cleanup attribute above. return nullptr; } diff --git a/src/detection/top/top.c b/src/detection/top/top.c index a39aeb8a1..9ff753336 100644 --- a/src/detection/top/top.c +++ b/src/detection/top/top.c @@ -7,24 +7,12 @@ static double startTick; static FFTopTypes preparedTypes; void ffPrepareTopProcesses(FFTopTypes showTypes) { - if ((showTypes & (FF_TOP_TYPE_CPU | FF_TOP_TYPE_DISK)) == 0) { - return; // Memory usage is instantaneous; no baseline snapshot is needed + if (startTick != 0) { + return; // Already prepared } - if (startTick != 0) { - // Already prepared. The baseline above is one snapshot shared by every `top` module in the - // run, so a second module asking for a different set of counters would be measured against a - // snapshot that never collected them: its difference comes out as the process lifetime - // average rather than a rate, and the `newItem->... < oldItem->...` checks in - // `ffDetectTopProcesses` cannot tell, because the missing counters read as 0 there. This is - // a configuration fastfetch cannot serve, so it is refused rather than printed wrong. - if (showTypes != preparedTypes) { - fputs("Error: `top` modules with different `showTypes` cannot share a run\n", stderr); - fputs(" The baseline snapshot is collected once and reused, so it only holds the counters the first module asked for.\n", stderr); - fputs(" Give the modules the same `showTypes`, or run them in separate invocations.\n", stderr); - exit(1); - } - return; + if ((showTypes & (FF_TOP_TYPE_CPU | FF_TOP_TYPE_DISK)) == 0) { + return; // Memory usage is instantaneous; no baseline snapshot is needed } // Within one module `showTypes` cannot change between this call and `ffDetectTopProcesses`: @@ -81,6 +69,13 @@ const char* ffDetectTopProcesses(FFTopOptions* options, FFlist* result) { return nullptr; } + // A baseline collected for a different set of counters is answered here rather than in + // ffPrepareTopProcesses, which has no way to report an error to the caller. The module that + // triggered the mismatch is the one that sees it; see ffPrepareTopProcesses. + if (options->showTypes != preparedTypes && (options->showTypes & (FF_TOP_TYPE_CPU | FF_TOP_TYPE_DISK)) != 0) { + return "`top` modules with different `showTypes` cannot share a run"; + } + // Memory usage and thread count are instantaneous; when neither CPU time nor disk IO // counters are requested, a single snapshot suffices and no sampling wait is needed. const bool sampleOnce = (options->showTypes & (FF_TOP_TYPE_CPU | FF_TOP_TYPE_DISK)) == 0; diff --git a/src/detection/top/top_linux.c b/src/detection/top/top_linux.c index a67dc04f0..70b7466c9 100644 --- a/src/detection/top/top_linux.c +++ b/src/detection/top/top_linux.c @@ -68,7 +68,7 @@ static bool parseStat(const char* buffer, size_t length, FFTopProcessSnapshot* r } const char* ffTopGetProcessSnapshot(FFlist* snapshots, FFTopTypes showTypes) { - const long ticks = sysconf(_SC_CLK_TCK); + const long ticks = sysconf(_SC_CLK_TCK); // sysconf returns -1 ONLY if `_SC_CLK_TCK` is an invalid name const uint32_t pageSizeShift = instance.state.platform.sysinfo.pageSizeShift; FF_DEBUG("Scanning /proc: clk_tck=%ld, pageSizeShift=%u", ticks, (unsigned) pageSizeShift); diff --git a/src/detection/wallpaper/wallpaper_android.c b/src/detection/wallpaper/wallpaper_android.c index 5f89d74de..33735a643 100644 --- a/src/detection/wallpaper/wallpaper_android.c +++ b/src/detection/wallpaper/wallpaper_android.c @@ -49,9 +49,17 @@ typedef enum FFWallpaperAndroidWhich : int32_t { // methods as it likes -- this one dropped `getName`, whose code answers UNKNOWN_TRANSACTION here. #define FF_WALLPAPER_ANDROID_TRANSACTION_GET_WALLPAPER 4u -// The request is the interface token, the package name, `which` and `userId`. The package name is the -// longest thing in it, and the helper will not write more than 128 bytes of it. -#define FF_WALLPAPER_ANDROID_PARCEL_SIZE 256 +// `ffAndroidGetOwnPackage` writes at most this many bytes, terminator included. +#define FF_WALLPAPER_ANDROID_PACKAGE_SIZE 128 + +// The request is the interface token, the package name, `which` and `userId`. A string16 costs +// 4 + 2 * (length + 1) bytes padded to four, and the token puts three int32 in front of the +// descriptor. Deriving the size from the two names rather than rounding it up is what keeps the +// longest package name `ffAndroidGetOwnPackage` can produce from marking the parcel truncated -- +// that fails the whole detection, rather than degrading to a call without a name. +#define FF_WALLPAPER_ANDROID_PARCEL_SIZE \ + (12 + ((4 + 2 * sizeof(FF_WALLPAPER_ANDROID_DESCRIPTOR) + 3) & ~3u) \ + + ((4 + 2 * FF_WALLPAPER_ANDROID_PACKAGE_SIZE + 3) & ~3u) + 8) // The reply carries the descriptor object plus, for this method, a Bundle of two ints: 108 bytes on // the device it was measured on, 80 when the wallpaper is a live one and the Bundle comes alone. Only @@ -134,7 +142,7 @@ const char* ffDetectWallpaper(FFstrbuf* result) { return ffDetectWallpaperLinux(result); } - char package[128]; + char package[FF_WALLPAPER_ANDROID_PACKAGE_SIZE]; if (!ffAndroidGetOwnPackage(package, sizeof(package))) { return "Cannot determine the package name of this process"; } diff --git a/src/detection/wallpaper/wallpaper_apple.m b/src/detection/wallpaper/wallpaper_apple.m index b736ee864..76d03168e 100644 --- a/src/detection/wallpaper/wallpaper_apple.m +++ b/src/detection/wallpaper/wallpaper_apple.m @@ -23,9 +23,16 @@ const char* detectFromPlist(FFstrbuf* result) { NSDictionary* choice = choices[0]; NSArray* files = choice[@"Files"]; - if (files.count > 0) { - NSString* file = files[0][@"relative"]; - ffStrbufSetS(result, [NSURL URLWithString:file].path.UTF8String); + if ([files isKindOfClass:NSArray.class] && files.count > 0) { + NSDictionary* fileEntry = files[0]; + // `relative` is read through a dictionary, which raises an unrecognized selector rather + // than returning nil when the entry is not one. + if ([fileEntry isKindOfClass:NSDictionary.class]) { + NSString* relative = fileEntry[@"relative"]; + if ([relative isKindOfClass:NSString.class]) { + ffStrbufSetS(result, [NSURL URLWithString:relative].path.UTF8String); + } + } } if (result->length == 0) { @@ -127,6 +134,11 @@ const char* ffDetectWallpaper(FFstrbuf* result) { } else { #ifdef FF_HAVE_SQLITE3 error = detectFromSQLite(result); + if (error) { + // The database only holds the user-picked picture, so it goes missing on a machine that + // never changed its wallpaper. NSWorkspace answers for the system default there. + error = detectFromNSWorkspace(result); + } #else error = detectFromNSWorkspace(result); #endif diff --git a/src/detection/wifi/wifi_android.c b/src/detection/wifi/wifi_android.c index 3cb2a4377..0da49ad94 100644 --- a/src/detection/wifi/wifi_android.c +++ b/src/detection/wifi/wifi_android.c @@ -58,15 +58,29 @@ // the fields the caller may not see removed -- `WifiInfo.makeCopy` drops the SSID, writes // `02:00:00:00:00:00` in place of the BSSID and the MAC address, and rewrites the network id to -1, // while the signal, the rates, the frequency, the IPv4 address and the supplicant state are carried -// over untouched. It makes that copy for every caller that `enforceCanAccessScanResults` refuses, -// which was measured here as a caller whose location permission had lapsed, next to the shell, which -// holds one and reads the SSID. Nothing else about the reply moves, so the head is understood the -// same way and everything the reply still says is reported: a connection that cannot be named is -// still a connection, and the name is the only thing missing. The two names are reported as -// `` -- the same word the macOS backend uses for the same situation -- rather than as -// nothing, because an empty SSID is what the module prints the interface state in place of, and the -// reply did carry a signal, a channel and a rate worth showing. The placeholders the service writes -// in their place are never reported as values. See parseConnectionInfo(). +// over untouched. It makes that copy for every caller that `enforceCanAccessScanResults` refuses. +// +// One route to a withheld name is a permission that can lapse. That copy is gated on location +// permission -- `enforceCanAccessScanResults` asks for `ACCESS_FINE_LOCATION` -- which is a runtime +// permission the user can revoke at any time, so a caller that once read the SSID keeps its Wi-Fi +// permission and stops being told the name. Measured here as a caller whose location permission had +// lapsed, next to the shell, which holds one and reads the SSID. Nothing else about the reply moves, +// so the head is understood the same way and everything the reply still says is reported: a +// connection that cannot be named is still a connection, and the name is the only thing missing. +// The two names are reported as `` -- the same word the macOS backend uses for the same +// situation -- rather than as nothing, because an empty SSID is what the module prints the interface +// state in place of, and the reply did carry a signal, a channel and a rate worth showing. The +// placeholders the service writes in their place are never reported as values. See +// parseConnectionInfo(). +// +// A UID with no Wi-Fi permission at all takes a different route, and it is worth naming because it +// is the one that looks like this one from the outside. There the first call raises EX_SECURITY (-1) +// and the module fails outright: nothing is printed, `` never appears, and the caller sees +// "The Wifi service raised an exception". The reason is that the caller's UID is what the service +// checks, so no later field of a reply that was never delivered can be recovered -- the two routes +// are not a fallback chain. There is no degraded answer to give here and no route left in this file: +// without the permission the service answers nothing at all, so the module cannot run rather than +// run worse. The file header above says which UID holds the permission and why. // // The network id goes with them, and that one matters to this file: -1 is what it reads as "there is // no connection", so a redacted reply would otherwise be reported as a disconnected one. What @@ -129,8 +143,9 @@ // from anywhere else -- sysfs and /proc/net are EACCES -- so the field stays empty rather than // naming an interface that was guessed at. `inf.status` is still reported, from the radio. // * `conn.ssid` and `conn.bssid` on a connection whose names the service withheld are reported as -// `` instead, which is not empty: see the note above. They are left empty when there -// is no connection at all, where an absent name is the truth rather than a redaction. +// `` instead, which is not empty: see the note above. They are the two fields a +// lapsed location permission takes away and nothing else. They are left empty when there is no +// connection at all, where an absent name is the truth rather than a redaction. #define FF_WIFI_ANDROID_SERVICE "wifi" #define FF_WIFI_ANDROID_DESCRIPTOR "android.net.wifi.IWifiManager" @@ -634,7 +649,14 @@ static const char* parseConnectionInfo(const uint8_t* data, size_t size, FFWifiA } const int32_t exception = ffBinderReadI32(data, size, 0); if (exception != 0) { - FF_DEBUG("The Wifi service raised exception %d", exception); + // The two exceptions this call raises mean different things to a reader, and the code below + // can not tell them apart -- it only sees that the connection could not be read. -1 is + // EX_SECURITY, which is the service refusing a UID that does not hold ACCESS_WIFI_STATE, and + // that is the whole module failing rather than one field going missing. The reply with the + // names withheld is the *other* route: it is a successful reply, and it is handled below as + // a connection that can not be named, not as an error. Saying so here is what keeps the two + // from reading as one. + FF_DEBUG("The Wifi service raised exception %d%s", exception, exception == -1 ? " (EX_SECURITY): this UID has no ACCESS_WIFI_STATE, so no reply carries a connection" : ""); return "The Wifi service raised an exception"; } if (ffBinderReadU32(data, size, 4) == 0) { @@ -792,9 +814,12 @@ static double wifiAndroidRate(int32_t specific, int32_t generic) { // The SSID and the BSSID as the module reports them: the value when the reply carried one, and // `` when the service withheld it. A withheld name is not reported as nothing, because an // empty SSID is what the module prints the interface state in place of -- and the reply did carry a -// signal, a channel and a rate that are worth showing. `withheld` is only ever true for a connection: -// the placeholder the service writes in place of an address is also what an empty `WifiInfo` carries, -// but a reply with no connection never reaches the point where the names are set. +// signal, a channel and a rate that are worth showing. This is the whole of the degradation: what a +// lapsed location permission costs the module is these two fields and nothing else, since the +// service withholds them from the copy it makes and leaves the rest of the reply intact. +// `withheld` is only ever true for a connection: the placeholder the service writes in place of an +// address is also what an empty `WifiInfo` carries, but a reply with no connection never reaches the +// point where the names are set. static const char* wifiName(const char* value, bool withheld) { if (value[0] != '\0') { return value; diff --git a/src/logo/image/image.c b/src/logo/image/image.c index ce130ede3..e0cffbfbe 100644 --- a/src/logo/image/image.c +++ b/src/logo/image/image.c @@ -1000,7 +1000,11 @@ static FFNativeFD getCacheFD(FFLogoRequestData* requestData, const char* fileNam #endif ); #else - HANDLE fd = CreateFileA(requestData->cacheDir.chars, GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + wchar_t widePath[PATH_MAX]; + if (!NT_SUCCESS(RtlUTF8ToUnicodeN(widePath, sizeof(widePath), nullptr, requestData->cacheDir.chars, requestData->cacheDir.length + 1))) { + return nullptr; + } + HANDLE fd = CreateFileW(widePath, GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); #endif ffStrbufSubstrBefore(&requestData->cacheDir, cacheDirLength); return fd; @@ -1261,6 +1265,15 @@ static uint32_t getKittyImageId(void) { // of its own, so its gap is set with a separate `a=a` command; every further frame is an `a=f` // command that replaces the pixels (`X=1`) because the backend already composed a full canvas. // Every command carries the image id and `q=2`, so nothing is ever written back to the tty. +// +// `z` is a non-negative millisecond gap, so the backend's -1 ("gapless", see FFImageFrame) has to +// be clamped: emitting it verbatim is out of range, and a terminal that validates the key drops +// the whole command rather than the one key. -1 and 0 mean the same thing to a terminal that does +// not validate. +static int32_t getKittyGap(int32_t delayMs) { + return delayMs > 0 ? delayMs : 0; +} + static void emitKittyAnimation(FFstrbuf* result, const FFKittyAnimation* animation, uint32_t imageId) { const char* currentPos = animation->payload; @@ -1277,7 +1290,7 @@ static void emitKittyAnimation(FFstrbuf* result, const FFKittyAnimation* animati // the terminal sizes the frame's canvas from them and rejects the command outright when // they are missing. ffStrbufAppendF(result, "\033_Ga=f,f=32,s=%u,v=%u,i=%u,z=%d,X=1,q=2", - animation->width, animation->height, imageId, (int) frame->delayMs); + animation->width, animation->height, imageId, (int) getKittyGap(frame->delayMs)); } if (frame->compressed) { @@ -1293,7 +1306,7 @@ static void emitKittyAnimation(FFstrbuf* result, const FFKittyAnimation* animati if (i == 0) { // "the first frame or root frame is created with the base image data and has no gap, // so its gap must be set using this control code" - ffStrbufAppendF(result, "\033_Ga=a,i=%u,r=1,z=%d,q=2\033\\", imageId, (int) frame->delayMs); + ffStrbufAppendF(result, "\033_Ga=a,i=%u,r=1,z=%d,q=2\033\\", imageId, (int) getKittyGap(frame->delayMs)); } } @@ -1348,8 +1361,13 @@ static bool encodeKittyAnimation(FFLogoRequestData* requestData, const char** er const int32_t loopCount = ffImageAnimationLoopCount(animation); if (frameCount == 1) { - // Nothing to animate. Rendering it through the static path keeps the cache entry, and - // therefore the bytes written to the terminal, identical to the default rendering. + // Nothing to animate, but the caller asked for an animation and that is what the cache has to + // hold: the reader for an animation never falls back to a still entry (see ยง7.1), so a + // `kittyanim` is what it looks for whoever wrote one. Writing the still pair instead would + // leave the entry functionally complete -- the mtime goes down, removeCachedFiles() is + // skipped from then on -- while every run re-decodes and re-encodes the source, because the + // entry the reader wants is missing. One frame, encoded the same way an animation of one + // frame would be, keeps both the bytes sent and the cache honest. FFImageFrame frame = {}; bool ok = ffImageAnimationGetFrame(animation, 0, &frame, error); ffImageAnimationClose(animation); @@ -1362,9 +1380,47 @@ static bool encodeKittyAnimation(FFLogoRequestData* requestData, const char** er .width = requestData->logoPixelWidth, .height = requestData->logoPixelHeight, }; - ok = printImageKitty(requestData, &buffer); + + const size_t frameSize = (size_t) requestData->logoPixelWidth * requestData->logoPixelHeight * 4; + FF_AUTO_FREE void* blob = malloc(frameSize); + if (blob == nullptr) { + ffImageDestroy(&buffer); + *error = "out of memory"; + return false; + } + memcpy(blob, frame.data, frameSize); + + size_t blobLength = frameSize; + bool compressed = false; + #ifdef FF_HAVE_ZLIB + compressed = compressBlob(&blob, &blobLength); + #endif + + FF_STRBUF_AUTO_DESTROY base64 = ffStrbufCreateA((uint32_t) (10 + blobLength * 4 / 3)); + ffBase64EncodeRaw((uint32_t) blobLength, (const char*) blob, &base64.length, base64.chars); + + FFKittyAnimationFrame single = { .delayMs = frame.delayMs, .compressed = compressed, .payloadLength = base64.length }; + FFKittyAnimation built = { + .width = requestData->logoPixelWidth, + .height = requestData->logoPixelHeight, + .frameCount = 1, + .loopCount = loopCount, + .frames = &single, + .payload = base64.chars, + .payloadLength = base64.length, + }; + + FF_STRBUF_AUTO_DESTROY serialized = ffStrbufCreate(); + serializeKittyAnimation(&serialized, &built); + writeCacheData(requestData, serialized.chars, serialized.length, FF_CACHE_FILE_KITTY_ANIMATION); + + // The envelope carries the image id, so the bytes handed to the terminal are not the cached + // ones and go out without a payload write. The still pair is written by printImageKitty -- + // it is what a still selector reads, and this frame is the frame every still selector asks + // for. It runs before the buffer is released for that reason. + printImageKitty(requestData, &buffer); ffImageDestroy(&buffer); - return ok; + return true; } FFKittyAnimationFrame* frames = calloc(frameCount, sizeof(*frames)); diff --git a/src/logo/image/wic.cpp b/src/logo/image/wic.cpp index d2ca3c4f7..c5a3520c7 100644 --- a/src/logo/image/wic.cpp +++ b/src/logo/image/wic.cpp @@ -122,15 +122,28 @@ static bool convertToRGBA(IWICImagingFactory* factory, IWICBitmapSource* source, return false; } - UINT stride = outWidth * 4; - uint8_t* pixels = (uint8_t*) malloc((size_t) stride * outHeight); + // `stride` and the length WIC is told are both UINT, while the buffer is a size_t allocation, so + // computing either in UINT would let a large request wrap and have WIC fill a buffer it believes + // is smaller than it is. Not reachable today -- the scaler above rejects any dimension above + // 32767, which caps the stride at 131068 and the buffer at 4294705156, both inside UINT -- but + // the guard costs nothing and every other backend keeps its buffer size associative. Kept in + // size_t throughout so the check also covers the allocation overflowing size_t. + const size_t strideSize = (size_t) outWidth * 4; + const size_t bufferSize = strideSize * outHeight; + if (strideSize > UINT_MAX || bufferSize > UINT_MAX) { + if (error) *error = "the requested image dimensions are too large"; + return false; + } + const UINT stride = (UINT) strideSize; + + uint8_t* pixels = (uint8_t*) malloc(bufferSize); if (pixels == nullptr) { if (error) *error = "out of memory"; return false; } // prc == nullptr means the whole image; WIC fills the buffer using the stride we pass in - if (FAILED(converter->CopyPixels(nullptr, stride, stride * outHeight, pixels))) { + if (FAILED(converter->CopyPixels(nullptr, stride, (UINT) bufferSize, pixels))) { free(pixels); if (error) *error = "pixel copy failed"; return false; diff --git a/src/modules/battery/battery.c b/src/modules/battery/battery.c index bd98043d4..57c4d5666 100644 --- a/src/modules/battery/battery.c +++ b/src/modules/battery/battery.c @@ -19,7 +19,7 @@ static void printBattery(FFBatteryOptions* options, FFBatteryResult* result, uin ffStrbufClear(&key); FF_PARSE_FORMAT_STRING_CHECKED(&key, &options->moduleArgs.key, ((FFformatarg[]) { FF_ARG(index, "index"), - FF_ARG(result->modelName, "name"), + FF_ARG(result->modelName, "model-name"), FF_ARG(options->moduleArgs.keyIcon, "icon"), FF_ARG(FF_MODULE_GET_DISPLAY_NAME(Battery), "module-name"), })); @@ -147,7 +147,7 @@ static void printBattery(FFBatteryOptions* options, FFBatteryResult* result, uin FF_PRINT_FORMAT_CHECKED(key.chars, 0, &options->moduleArgs, FF_PRINT_TYPE_NO_CUSTOM_KEY, ((FFformatarg[]) { FF_ARG(result->manufacturer, "manufacturer"), - FF_ARG(result->modelName, "name"), + FF_ARG(result->modelName, "model-name"), FF_ARG(result->technology, "technology"), FF_ARG(capacityNum, "capacity"), FF_ARG(status, "status"), @@ -330,7 +330,7 @@ FFModuleBaseInfo ffBatteryModuleInfo = { .generateJsonConfig = (void*) ffGenerateBatteryJsonConfig, .formatArgs = FF_FORMAT_ARG_LIST(((FFModuleFormatArg[]) { { "Battery manufacturer", "manufacturer" }, - { "Battery model name *", "name" }, + { "Battery model name *", "model-name" }, { "Battery technology", "technology" }, { "Battery capacity (percentage num)", "capacity" }, { "Battery status", "status" }, diff --git a/src/modules/poweradapter/poweradapter.c b/src/modules/poweradapter/poweradapter.c index bdd79e455..3780ba103 100644 --- a/src/modules/poweradapter/poweradapter.c +++ b/src/modules/poweradapter/poweradapter.c @@ -142,7 +142,7 @@ FFModuleBaseInfo ffPowerAdapterModuleInfo = { { "Power adapter watts", "watts" }, { "Power adapter name", "name" }, { "Power adapter manufacturer", "manufacturer" }, - { "Power adapter model", "model-name" }, + { "Power adapter model name", "model-name" }, { "Power adapter description", "description" }, { "Power adapter serial number", "serial" }, })),