[PATCH v4 01/15] x86: Remove unused SFrame CFI RA register variable

Jens Remus jremus@linux.ibm.com
Mon Jun 24 16:13:24 GMT 2024


Hello Jan!

Am 24.06.2024 um 16:51 schrieb Jan Beulich:
> On 24.06.2024 16:23, Jens Remus wrote:
>> gas/
>> 	* config/tc-i386.c: Remove unused SFrame CFI RA register
>> 	variable.
> 
> Nit: The more "canonical" way would be
> 
> 	* config/tc-i386.c (x86_sframe_cfa_ra_reg): Remove.
> 

Thanks! I am still learning how to get those GNU Changelog entries in 
the commit message correct. While this series evolved later added 
patches do have the function names. Somehow I did not consider to reword 
them all.

I will reword the commit message as you suggested. Do you want me to 
send a v5 (after Indu reviewed the whole series)?

>> Reviewed-by: Andreas Krebbel <krebbel@linux.ibm.com>
>> Reviewed-by: Indu Bhagat <indu.bhagat@oracle.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> 
> As to these tags, besides them wanting to be in chronological order,
> would you mind pointing out where these were taken from? I've checked
> and I have no record of having offered an Acked-by:, and I also
> couldn't find any Reviewed-by anywhere on the list. I don't know how
> bad it really is (so please forgive if the wording is stronger than
> actually needed here), but I don't think tags should be falsified. I
> may be entirely wrong, though.

You concerns are fair. Let me try to clarify. I am used to the Linux 
kernel style commit message trailers from my other short prior 
open-source development work experience. To best adhere to the GNU 
Binutils push after approval rule my intention was to record any reviews 
and the required approval. Given those tailers are not widely used on 
the GNU Binutils mailing list (this seems different for GDB [2]) I took 
some freedom translating the feedback received to my patches. If this 
was wrong I apologize for my naive approach.

Andreas is my co-worker and GNU Binutils maintainer for s390. He had 
reviewed an early version of this patch series including this specific 
patch as part of an internal-review. This is the source of his Reviewed-by.

Indu had reviewed the patch in 
<ebeb4ed6-23df-468c-985b-fb47a3fede00@oracle.com> 
(https://sourceware.org/pipermail/binutils/2024-February/132653.html). I 
took the freedom to translate her "LGTM" to "Reviewed-by".

You reviewed the patch in 
<55490c94-bb76-4c23-8214-bfb8a9d193e2@suse.com> 
(https://sourceware.org/pipermail/binutils/2024-February/132669.html) 
and <a10708f0-62ec-4ecd-a200-baef78102f63@suse.com> 
(https://sourceware.org/pipermail/binutils/2024-February/132686.html). 
Since you are one of the GNU Binutils maintainers for x86 I translated 
your "Okay. So Jens - feel free to put in." from the latter to 
"Acked-by", since I assumed this to be you ok to push for this 
particular patch.

Somehow I am used to keep my own Signed-off-by trailer last, to denote 
that I confirm any above trailers.

> 
> With the (tag free) replies you've got, I don't think there would
> have been a need to re-post anyway - you could simply have committed
> the adjusted form.

I can remove all of the trailers and stop adding them, if that is the 
preferred approach for GNU Binutils. I honestly thought nobody would 
feel hurt if I used them as explained above.
I must admit that I felt a bit uncomfortable not having them. Thinking 
that through again, I come to the conclusion that they are not really 
adding much value, since they don't include any reference to their 
origin.  To not loose track of which patches are already 
reviewed/approved I could track the progress in my private Git notes 
instead.

> 
> Finally the title says 01/15, yet by now (about 20 min after the mail
> arrived) I didn't get any further parts of such a 15-patch series.
> Oddly enough [1] doesn't even have this one, yet I don't know how
> often it would be refreshed.

I have the impression my e-mails to the GNU Binutils mailing list are 
lately withheld for some reason. At least my recent patch series "[PATCH 
0/2] aarch64: Fixes access to struct aarch64_opnd_info members" from 
2024-06-21 did only show up the next day on the mailing list for me. 
Since you are on Cc only for this one x86-specifc patch, you got it 
earlier than the rest of the series, which should arrive soon.

> 
> I'm sorry for the ranting,
> Jan

No problem, this is absolutely fine! I prefer to hear any concerns early 
and learn and adapt, than to continue doing things wrong.

> 
> [1] https://sourceware.org/pipermail/binutils/2024-June/thread.html
> 

[2] GDB Contribution Checklist, section "12. Receiving positive reviews",
 
https://sourceware.org/gdb/wiki/ContributionChecklist#Receiving_positive_reviews

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303) and z/VSE Support
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des 
Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der 
Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/


More information about the Binutils mailing list