On 12/03/2025 13:54, Yi Liu wrote:
> Caution: External email. Do not open attachments or click links, unless
> this email comes from a known sender and you know the content is safe.
>
>
> On 2025/3/12 18:03, Philippe Mathieu-Daudé wrote:
>> 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.
>
> yeah, that might be sadly possible. :(
>
>>>
>>> Now, accidents happen, no big deal, etc., etc. I post this to hopefully
>>> help reduce the accident rate :)
>
> a dumb question. Where can I view this issue?
>
>>>
>>> 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?
>
> Commit eda4c9b5b3c is the similar case. Zhenzhong and Clément took
> the patch from me and I was cced when Zhenzhong sent it out. I gave
> my r-b after reviewing it.
Some other commits of the same series were in a similar situation:
initially written by me and slightly changed by Zhenzhong.
These are not caught by one-liner above because I deliberately didn't
give an rb.
According to Daniel it seems to be ok to review a co-authored patch but
is this considered a last resort?
>
> --
> Regards,
> Yi Liu
