Hi Markus, (Cc'ing Yi, Clément and Zhenzhong for commit eda4c9b5b3c)
On 12/3/25 10:45, Markus Armbruster wrote:
I stumbled over commits that carry the author's Reviewed-by. There may be cases where the recorded author isn't the lone author, and the recorded author did some meaningful review of the patch's parts that are not theirs. Mind that we do need all authors to provide their Signed-off-by. When the only Signed-off-by is from the recorded author, and there's also their Reviewed-by, the Reviewed-by is almost certainly bogus. Now, accidents happen, no big deal, etc., etc. I post this to hopefully help reduce the accident rate :) Here's my quick & sloppy search for potentially problematic uses of Reviewed-by: $ git-log --since 'two years ago' | awk -F: '/^commit / { commit=$0 } /^Author: / { guy=$2 } /^ Reviewed-by: / { if ($2 == guy) { print commit; print guy } }'
Explaining some commits where I'm mentioned: commit 1e0d4eb4ee7c909323bffc39bc348eb3174b426b Author: Philippe Mathieu-Daudé <phi...@linaro.org> Date: Fri Apr 12 00:33:30 2024 -0700 backends/tpm: Use qemu_hexdump_line() to avoid sprintf() sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1. Using qemu_hexdump_line() both fixes the deprecation warning and simplifies the code base. Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> Reviewed-by: Stefan Berger <stef...@linux.ibm.com> [rth: Keep the linebreaks every 16 bytes] Signed-off-by: Richard Henderson <richard.hender...@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> Message-ID: <20240412073346.458116-12-richard.hender...@linaro.org> [PMD: Rebased] I posted a patch with my S-o-b; Richard took it, improved and reposted it with his S-o-b; I reviewed Richard's changes (and eventually merged). commit 0fe4cac5dda1028c22ec3a6997e1b9155a768004 Author: Peter Maydell <peter.mayd...@linaro.org> Date: Mon Jul 17 18:29:40 2023 +0200 target/mips: Avoid shift by negative number in page_table_walk_refill() Coverity points out that in page_table_walk_refill() we can shift by a negative number, which is undefined behaviour (CID 1452918, 1452920, 1452922). We already catch the negative directory_shift and leaf_shift as being a "bail out early" case, but not until we've already used them to calculated some offset values. The shifts can be negative only if ptew > 1, so make the bail-out-early check look directly at that, and only calculate the shift amounts and the offsets based on them after we have done that check. This allows us to simplify the expressions used to calculate the shift amounts, use an unsigned type, and avoids the undefined behaviour. Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> [PMD: Check for ptew > 1, use unsigned type] Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> Message-Id: <20230717213504.24777-3-phi...@linaro.org> Peter posted the first patch, I reworked it and reposted, Peter reviewed my changes. commit c4380f7bcdcb68fdfca876db366782a807fab8f7 Author: Richard Henderson <richard.hender...@linaro.org> Date: Thu Jan 18 21:06:30 2024 +0100 target/arm: Create arm_cpu_mp_affinity Wrapper to return the mp affinity bits from the cpu. Signed-off-by: Richard Henderson <richard.hender...@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> Reviewed-by: Richard Henderson <richard.hender...@linaro.org> Message-id: 20240118200643.29037-10-phi...@linaro.org Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> Is this workflow making sense and accepted? Otherwise what should we change? Maybe clarify along with the tags; or including all Message-Id could make this easier to track? Regards, Phil.