Skip to content

Commit

Permalink
elfnote: convert most usages of ELFNOTE_START() to ELFNOTE()
Browse files Browse the repository at this point in the history
These usages can be converted into one-liners.

Signed-off-by: Ilie Halip <[email protected]>
  • Loading branch information
ihalip committed Mar 5, 2020
1 parent eee04ad commit c58ec05
Show file tree
Hide file tree
Showing 9 changed files with 9 additions and 27 deletions.
4 changes: 1 addition & 3 deletions arch/arm64/kernel/vdso/note.S
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
#include <linux/elfnote.h>
#include <linux/build-salt.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)

BUILD_SALT
4 changes: 1 addition & 3 deletions arch/mips/vdso/elf.S
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
#include <linux/elfnote.h>
#include <linux/version.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)

/*
* The .MIPS.abiflags section must be defined with the FP ABI flags set
Expand Down
4 changes: 1 addition & 3 deletions arch/nds32/kernel/vdso/note.S
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,4 @@
#include <linux/version.h>
#include <linux/elfnote.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)
4 changes: 1 addition & 3 deletions arch/s390/kernel/vdso64/note.S
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,4 @@
#include <linux/version.h>
#include <linux/elfnote.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)
4 changes: 1 addition & 3 deletions arch/sparc/vdso/vdso-note.S
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@
#include <linux/version.h>
#include <linux/elfnote.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)
4 changes: 1 addition & 3 deletions arch/sparc/vdso/vdso32/vdso-note.S
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@
#include <linux/version.h>
#include <linux/elfnote.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)
4 changes: 1 addition & 3 deletions arch/x86/entry/vdso/vdso-note.S
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#include <linux/version.h>
#include <linux/elfnote.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)

BUILD_SALT
4 changes: 1 addition & 3 deletions arch/x86/entry/vdso/vdso32/note.S
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
/* Ideally this would use UTS_NAME, but using a quoted string here
doesn't work. Remember to change this when changing the
kernel's name. */
ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)

BUILD_SALT

Expand Down
4 changes: 1 addition & 3 deletions arch/x86/um/vdso/vdso-note.S
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,4 @@
#include <linux/version.h>
#include <linux/elfnote.h>

ELFNOTE_START(Linux, 0)
.long LINUX_VERSION_CODE
ELFNOTE_END
ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)

4 comments on commit c58ec05

@nickdesaulniers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So whenever I see lots of repetition, like ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE) I think "don't repeat yourself" (aka DRY principle).

We could convert all these to also implicitly pass Linux, 0, .long LINUX_VERSION_CODE, just like you did for the previous patch in this series.

It looks like just arch/x86/entry/vdso/vdso32/note.S sets ELFNOTE_START(GNU, 2, "a"). Maybe you can dig into the context of that change and see if it's still necessary; if it's not, than removing that first would allow for more aggressive and DRY'er refactoring.

@ihalip
Copy link
Owner Author

@ihalip ihalip commented on c58ec05 Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GNU libc supports pseudo-hwcaps which is loads from the vDSO's type 2 GNU note. The process is documented a bit in https://sourceware.org/git/?p=glibc.git;a=commit;h=141af660d825d4443cbc5c24d29d57d6f8b0950f.
Looks like this feature is still used by Xen to load some optimized libraries, though at least Red Hat removed it for 32-bit Xen in RHEL8.

@nickdesaulniers
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great sleuthing. In that case, why don't we split the macro in two, or create a new that we will use more often?

#define ELFNOTE_LINUX ELFNOTE(Linux, 0, .long LINUX_VERSION_CODE)

Then you can just use ELFNOTE_LINUX everywhere, and the lone type 2 GNU note doesn't have to change. That way the common case is more concise.

Thoughts?

@ihalip
Copy link
Owner Author

@ihalip ihalip commented on c58ec05 Mar 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would avoid including version.h directly in elfnote.h for just a macro like that. What about something like this instead?

#define ELFNOTE_LINUX ELFNOTE(Linux, 0, desc)

And its usage would be like this:

ELFNOTE_LINUX(.long LINUX_VERSION_CODE)

Please sign in to comment.