From f8fd708423094979e4b242a061aecd1f46d2f87c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E9=80=9A=E6=B4=B2?= Date: Thu, 17 Sep 2026 19:19:25 +0800 Subject: [PATCH] Global: addresses Copilot's review comments --- doc/json_schema.json | 5 ++++ src/common/endian.h | 7 ++++- src/common/impl/FFPlatform_unix.c | 36 +++++++++++++++++++---- src/detection/top/top_apple.c | 14 +++++++-- src/detection/wallpaper/wallpaper_apple.m | 5 ++++ src/logo/image/im6.c | 23 +++++++++++++++ 6 files changed, 82 insertions(+), 8 deletions(-) diff --git a/doc/json_schema.json b/doc/json_schema.json index d64c5796b..03ba42ece 100644 --- a/doc/json_schema.json +++ b/doc/json_schema.json @@ -790,6 +790,11 @@ ], "default": "left" }, + "animationFrame": { + "type": "integer", + "description": "Frame of an animated image to render:\n0 plays the animation (kitty image protocol only), a positive value is the 1-based frame number, and a negative value counts back from the end, so -1 is the last frame.\nOnly the JSON config can express a negative value, as the command line parser reads a leading `-` as another option", + "default": 1 + }, "chafa": { "type": "object", "additionalProperties": false, diff --git a/src/common/endian.h b/src/common/endian.h index 2410911ee..78de3aa3b 100644 --- a/src/common/endian.h +++ b/src/common/endian.h @@ -2,7 +2,12 @@ #include -#if __BIG_ENDIAN__ +// `__BIG_ENDIAN__` alone is not enough: GCC does not define it, it documents only `__BYTE_ORDER__` +// and the `__ORDER_*_ENDIAN__` values. Testing the former silently selected the little-endian +// branch on s390x, where `FF_READ_BE` then byte-swapped values that were already big-endian. +// `defined(__BIG_ENDIAN__)` alone would not do either: a macro that is defined as 0 is not a +// big-endian platform. +#if (defined(__BYTE_ORDER__) && __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) || (defined(__BIG_ENDIAN__) && __BIG_ENDIAN__) #define FF_READ_LE(x) _Generic((x), \ uint16_t: __builtin_bswap16(x), \ uint32_t: __builtin_bswap32(x), \ diff --git a/src/common/impl/FFPlatform_unix.c b/src/common/impl/FFPlatform_unix.c index 2c0ab9db3..0c9020691 100644 --- a/src/common/impl/FFPlatform_unix.c +++ b/src/common/impl/FFPlatform_unix.c @@ -102,14 +102,40 @@ static void getExePath(FFPlatform* platform) { int fileMib[6] = { CTL_KERN, KERN_FILE, KERN_FILE_BYPID, (pid_t) platform->pid, (int) sizeof(struct kinfo_file), 0 }; size_t fileSize = 0; if (sysctl(fileMib, ARRAY_SIZE(fileMib), nullptr, &fileSize, nullptr, 0) == 0) { - fileSize += fileSize / 8; // add ~10% - FF_AUTO_FREE struct kinfo_file* kf = (struct kinfo_file*) malloc(fileSize); + fileSize += fileSize / 8; // add 12.5% for the table growing between the two calls + size_t fileCapacity = fileSize; + FF_AUTO_FREE struct kinfo_file* kf = (struct kinfo_file*) malloc(fileCapacity); if (kf) { int rv; - do { - fileMib[5] = (int) (fileSize / sizeof(struct kinfo_file)); + // The table only grows, so the loop terminates as long as it grows by less + // than 12.5% per retry. Cap it anyway: spinning on a pathological growth + // rate would hang fastfetch, and giving up only costs the executable check + // below, which already assumes the path is correct when the list is missing. + int retries = 5; + while (true) { + fileMib[5] = (int) (fileCapacity / sizeof(struct kinfo_file)); + fileSize = fileCapacity; // sysctl takes the size of the buffer as input rv = sysctl(fileMib, ARRAY_SIZE(fileMib), kf, &fileSize, nullptr, 0); - } while (rv == -1 && errno == ENOMEM); + if (rv == 0 || errno != ENOMEM || retries-- == 0) { + break; + } + // The file table grew between the two calls. The buffer has to grow + // before the retry: the next call advertises fileCapacity as the size + // of kf, and the kernel would copy that many bytes into the old + // allocation. + // OpenBSD writes back the size it needs, FreeBSD only the number of + // bytes it managed to copy, so take the larger one and add ~10% on + // top -- the growth libprocstat uses for this very sysctl. + size_t newCapacity = fileSize > fileCapacity ? fileSize : fileCapacity; + newCapacity += newCapacity / 8; + struct kinfo_file* newKf = (struct kinfo_file*) realloc(kf, newCapacity); + if (newKf == nullptr) { + rv = -1; + break; + } + kf = newKf; + fileCapacity = newCapacity; + } if (rv == 0) { int cntp = (int) (fileSize / sizeof(struct kinfo_file)); diff --git a/src/detection/top/top_apple.c b/src/detection/top/top_apple.c index b37793e86..5d5c9208e 100644 --- a/src/detection/top/top_apple.c +++ b/src/detection/top/top_apple.c @@ -10,8 +10,14 @@ const char* ffTopGetProcessSnapshot(FFlist* snapshots, FFTopTypes showTypes) { if (npids <= 0) { return "proc_listallpids(nullptr, 0) failed"; } - FF_AUTO_FREE pid_t* pids = malloc((uint32_t) (npids + npids / 8 + 1) * sizeof(pid_t)); - npids = proc_listallpids(pids, npids); + // `proc_listallpids` returns the number of pids, but it wants the size of the buffer in bytes: + // passing the count back would only let the kernel fill a quarter of it. + const int pidCapacity = npids + npids / 8 + 1; + FF_AUTO_FREE pid_t* pids = malloc((size_t) pidCapacity * sizeof(pid_t)); + if (pids == nullptr) { + return "malloc() failed"; + } + npids = proc_listallpids(pids, pidCapacity * (int) sizeof(pid_t)); if (npids <= 0) { return "proc_listallpids(pids, bufferSize) failed"; } @@ -41,6 +47,10 @@ const char* ffTopGetProcessSnapshot(FFlist* snapshots, FFTopTypes showTypes) { item->startTime = proc.pbsd.pbi_start_tvsec * 1000u + proc.pbsd.pbi_start_tvusec / 1000u; // convert to ms item->threads = (uint32_t) proc.ptinfo.pti_threadnum; + // FF_LIST_ADD does not zero the element, and top.c subtracts both counters for every + // process it samples, whether or not they were collected. + item->bytesRead = 0; + item->bytesWritten = 0; if (showTypes & FF_TOP_TYPE_DISK) { struct rusage_info_v2 rusage; if (proc_pid_rusage(pid, RUSAGE_INFO_V2, (rusage_info_t*) &rusage) == 0) { diff --git a/src/detection/wallpaper/wallpaper_apple.m b/src/detection/wallpaper/wallpaper_apple.m index 1b4f6e3d5..9e9d86356 100644 --- a/src/detection/wallpaper/wallpaper_apple.m +++ b/src/detection/wallpaper/wallpaper_apple.m @@ -118,6 +118,11 @@ const char* ffDetectWallpaper(FFstrbuf* result) { if (@available(macOS 14.0, *)) { error = detectFromPlist(result); + if (error) { + // The plist is the authoritative source on Sonoma and later, but when it can not be read + // at all, NSWorkspace can still resolve a user-picked static image. + error = detectFromNSWorkspace(result); + } } else { #ifdef FF_HAVE_SQLITE3 error = detectFromSQLite(result); diff --git a/src/logo/image/im6.c b/src/logo/image/im6.c index 047f51905..8d5698e3c 100644 --- a/src/logo/image/im6.c +++ b/src/logo/image/im6.c @@ -3,6 +3,7 @@ #include "image.h" #include "common/library.h" #include "common/mallocHelper.h" + #include "common/strutil.h" #include #include @@ -98,6 +99,17 @@ static FFLogoImageResult im6EncodeImage(FFLogoRequestData* requestData, const ch ffCopyMagickString(imageInfoOut->magick, magick, magickLength); + // The raw pixel coders write image->depth bits per sample, not 8: a 1-bit grayscale source comes + // back as columns*4/8 bytes per row and a 16-bit one as columns*8, while the RGBA caller hands + // the blob on as RGBA8 and derives its length from width*height*4. Pin the depth for the raw + // formats only -- the SIXEL coder quantises on its own, so it keeps the source depth and its + // output stays byte identical. + // ImageMagick 6 leaves no choice about where to pin it: AcquireQuantumInfo takes the depth from + // image->depth and never looks at image_info->depth. + if (ffStrEquals(magick, "RGBA")) { + image->depth = 8; + } + blob = ffImageToBlob(imageInfoOut, image, &length, exceptionInfo); if (blob == nullptr || length == 0) { goto cleanup; @@ -142,6 +154,17 @@ bool ffImageCreateIM6(FFLogoRequestData* requestData, FFImageBuffer* out, const return false; } + // FFImageBuffer carries no length, so every consumer derives it from width*height*4. Refuse any + // other size rather than let them read past the blob -- the raw coder's depth scaling used to + // produce one (see the depth pin in im6EncodeImage). + if (length != (size_t) requestData->logoPixelWidth * requestData->logoPixelHeight * 4) { + if (error) { + *error = "Image Magick did not return an RGBA8 buffer"; + } + free(blob); + return false; + } + out->data = blob; out->width = requestData->logoPixelWidth; out->height = requestData->logoPixelHeight;