2020-04-22 10:36:56

by Ingo Molnar

[permalink] [raw]
Subject: [PATCH 2/3] objtool: Rename elf_read() to elf_open_read()

'struct elf *' handling is an open/close paradigm, make sure the naming
matches that:

elf_open_read()
elf_write()
elf_close()

Signed-off-by: Ingo Molnar <[email protected]>
---
tools/objtool/check.c | 2 +-
tools/objtool/elf.c | 2 +-
tools/objtool/elf.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f2a84271e807..12e2aea42bb2 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc)

objname = _objname;

- file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY);
+ file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);
if (!file.elf)
return 1;

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index fab5534c3365..453b723c89d5 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -542,7 +542,7 @@ static int read_relas(struct elf *elf)
return 0;
}

-struct elf *elf_read(const char *name, int flags)
+struct elf *elf_open_read(const char *name, int flags)
{
struct elf *elf;
Elf_Cmd cmd;
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index a55bcde9f1b1..5e76ac38cf99 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -113,7 +113,7 @@ static inline u32 rela_hash(struct rela *rela)
return sec_offset_hash(rela->sec, rela->offset);
}

-struct elf *elf_read(const char *name, int flags);
+struct elf *elf_open_read(const char *name, int flags);
struct section *elf_create_section(struct elf *elf, const char *name, size_t entsize, int nr);
struct section *elf_create_rela_section(struct elf *elf, struct section *base);
void elf_add_rela(struct elf *elf, struct rela *rela);
--
2.20.1


2020-04-22 12:24:36

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/3] objtool: Rename elf_read() to elf_open_read()

On Wed, Apr 22, 2020 at 12:32:04PM +0200, Ingo Molnar wrote:
> 'struct elf *' handling is an open/close paradigm, make sure the naming
> matches that:
>
> elf_open_read()
> elf_write()
> elf_close()


> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index f2a84271e807..12e2aea42bb2 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc)
>
> objname = _objname;
>
> - file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY);
> + file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);

Note that I have a patch pending that makes that unconditionally O_RDWR,
which sort of seems to suggest elf_open() might be the better name.

2020-04-22 14:26:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/3] objtool: Rename elf_read() to elf_open_read()


* Peter Zijlstra <[email protected]> wrote:

> On Wed, Apr 22, 2020 at 12:32:04PM +0200, Ingo Molnar wrote:
> > 'struct elf *' handling is an open/close paradigm, make sure the naming
> > matches that:
> >
> > elf_open_read()
> > elf_write()
> > elf_close()
>
>
> > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > index f2a84271e807..12e2aea42bb2 100644
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc)
> >
> > objname = _objname;
> >
> > - file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY);
> > + file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);
>
> Note that I have a patch pending that makes that unconditionally O_RDWR,
> which sort of seems to suggest elf_open() might be the better name.

Ok, done!

Thanks,

Ingo

2020-04-22 17:04:54

by Matt Helsley

[permalink] [raw]
Subject: Re: [PATCH 2/3] objtool: Rename elf_read() to elf_open_read()

On Wed, Apr 22, 2020 at 04:22:35PM +0200, Ingo Molnar wrote:
>
> * Peter Zijlstra <[email protected]> wrote:
>
> > On Wed, Apr 22, 2020 at 12:32:04PM +0200, Ingo Molnar wrote:
> > > 'struct elf *' handling is an open/close paradigm, make sure the naming
> > > matches that:
> > >
> > > elf_open_read()
> > > elf_write()
> > > elf_close()
> >
> >
> > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > > index f2a84271e807..12e2aea42bb2 100644
> > > --- a/tools/objtool/check.c
> > > +++ b/tools/objtool/check.c
> > > @@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc)
> > >
> > > objname = _objname;
> > >
> > > - file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY);
> > > + file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);
> >
> > Note that I have a patch pending that makes that unconditionally O_RDWR,
> > which sort of seems to suggest elf_open() might be the better name.
>
> Ok, done!

It might be a better name but there could be a problem with it --
see 8e144797f1a67c52e386161863da4614a23ad913
"objtool: Rename elf_open() to prevent conflict with libelf from elftoolchain"

Unless I'm forgetting something I think that'd still be an issue.

Cheers,
-Matt

2020-04-22 19:25:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/3] objtool: Rename elf_read() to elf_open_read()


* Matt Helsley <[email protected]> wrote:

> On Wed, Apr 22, 2020 at 04:22:35PM +0200, Ingo Molnar wrote:
> >
> > * Peter Zijlstra <[email protected]> wrote:
> >
> > > On Wed, Apr 22, 2020 at 12:32:04PM +0200, Ingo Molnar wrote:
> > > > 'struct elf *' handling is an open/close paradigm, make sure the naming
> > > > matches that:
> > > >
> > > > elf_open_read()
> > > > elf_write()
> > > > elf_close()
> > >
> > >
> > > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > > > index f2a84271e807..12e2aea42bb2 100644
> > > > --- a/tools/objtool/check.c
> > > > +++ b/tools/objtool/check.c
> > > > @@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc)
> > > >
> > > > objname = _objname;
> > > >
> > > > - file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY);
> > > > + file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);
> > >
> > > Note that I have a patch pending that makes that unconditionally O_RDWR,
> > > which sort of seems to suggest elf_open() might be the better name.
> >
> > Ok, done!
>
> It might be a better name but there could be a problem with it --
> see 8e144797f1a67c52e386161863da4614a23ad913
> "objtool: Rename elf_open() to prevent conflict with libelf from elftoolchain"
>
> Unless I'm forgetting something I think that'd still be an issue.

Indeed, that's a good point - I'll move back to elf_open_read()! :-)

Thanks,

Ingo

Subject: [tip: objtool/core] objtool: Rename elf_read() to elf_open_read()

The following commit has been merged into the objtool/core branch of tip:

Commit-ID: bc359ff2f6f3e8a9df38c39017e269bc442357c7
Gitweb: https://git.kernel.org/tip/bc359ff2f6f3e8a9df38c39017e269bc442357c7
Author: Ingo Molnar <[email protected]>
AuthorDate: Wed, 22 Apr 2020 12:32:04 +02:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Thu, 23 Apr 2020 08:34:18 +02:00

objtool: Rename elf_read() to elf_open_read()

'struct elf *' handling is an open/close paradigm, make sure the naming
matches that:

elf_open_read()
elf_write()
elf_close()

Acked-by: Josh Poimboeuf <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Sami Tolvanen <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
tools/objtool/check.c | 2 +-
tools/objtool/elf.c | 2 +-
tools/objtool/elf.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f2a8427..12e2aea 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2614,7 +2614,7 @@ int check(const char *_objname, bool orc)

objname = _objname;

- file.elf = elf_read(objname, orc ? O_RDWR : O_RDONLY);
+ file.elf = elf_open_read(objname, orc ? O_RDWR : O_RDONLY);
if (!file.elf)
return 1;

diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index fab5534..453b723 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -542,7 +542,7 @@ static int read_relas(struct elf *elf)
return 0;
}

-struct elf *elf_read(const char *name, int flags)
+struct elf *elf_open_read(const char *name, int flags)
{
struct elf *elf;
Elf_Cmd cmd;
diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
index a55bcde..5e76ac3 100644
--- a/tools/objtool/elf.h
+++ b/tools/objtool/elf.h
@@ -113,7 +113,7 @@ static inline u32 rela_hash(struct rela *rela)
return sec_offset_hash(rela->sec, rela->offset);
}

-struct elf *elf_read(const char *name, int flags);
+struct elf *elf_open_read(const char *name, int flags);
struct section *elf_create_section(struct elf *elf, const char *name, size_t entsize, int nr);
struct section *elf_create_rela_section(struct elf *elf, struct section *base);
void elf_add_rela(struct elf *elf, struct rela *rela);