2009-12-15 02:41:34

by Hatayama, Daisuke

[permalink] [raw]
Subject: [RFC, PATCH 2/4] elf_core_dump(): Remove DUMP_WRITE macro

In DUMP_WRITE macro, some local variables and goto statement are used
implicitly, which makes it confusing to understand how the dumping
process behaves. So, clean up it by unfolding the macro body directly
to the point where the original macro is used plus a bit of style
change.

Now that the previous patch replaced ELF_CORE_EXTRA_* macros in
fs/binfmt_elf.c, there is no users for DUMP_WRITE macro in
fs/binfmt_elf.c. Thus remove the definition of DUMP_WRITE macro.

Signed-off-by: Daisuke HATAYAMA <[email protected]>
---
fs/binfmt_elf.c | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5f7be51..9666a8a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1277,10 +1277,6 @@ static int writenote(struct memelfnote *men, struct file *file,
}
#undef DUMP_WRITE

-#define DUMP_WRITE(addr, nr) \
- if ((size += (nr)) > limit || !dump_write(file, (addr), (nr))) \
- goto end_coredump;
-
static void fill_elf_header(struct elfhdr *elf, int segs,
u16 machine, u32 flags, u8 osabi)
{
@@ -1981,7 +1977,10 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un
fs = get_fs();
set_fs(KERNEL_DS);

- DUMP_WRITE(elf, sizeof(*elf));
+ size += sizeof(*elf);
+ if (size > limit || !dump_write(file, elf, sizeof(*elf)))
+ goto end_coredump;
+
offset += sizeof(*elf); /* Elf header */
offset += (segs + 1) * sizeof(struct elf_phdr); /* Program headers */
foffset = offset;
@@ -1995,7 +1994,9 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un

fill_elf_note_phdr(&phdr, sz, offset);
offset += sz;
- DUMP_WRITE(&phdr, sizeof(phdr));
+ size += sizeof(phdr);
+ if (size > limit || !dump_write(file, &phdr, sizeof(phdr)))
+ goto end_coredump;
}

dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE);
@@ -2026,7 +2027,9 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un
phdr.p_flags |= PF_X;
phdr.p_align = ELF_EXEC_PAGESIZE;

- DUMP_WRITE(&phdr, sizeof(phdr));
+ size += sizeof(phdr);
+ if (size > limit || !dump_write(file, &phdr, sizeof(phdr)))
+ goto end_coredump;
}

if (!elf_core_write_extra_phdrs(file, offset, &size, limit))
--
1.6.5.1


2009-12-15 08:29:21

by Cong Wang

[permalink] [raw]
Subject: Re: [RFC, PATCH 2/4] elf_core_dump(): Remove DUMP_WRITE macro

On Tue, Dec 15, 2009 at 10:41 AM, Daisuke HATAYAMA
<[email protected]> wrote:
> In DUMP_WRITE macro, some local variables and goto statement are used
> implicitly, which makes it confusing to understand how the dumping
> process behaves. So, clean up it by unfolding the macro body directly
> to the point where the original macro is used plus a bit of style
> change.
>
> Now that the previous patch replaced ELF_CORE_EXTRA_* macros in
> fs/binfmt_elf.c, there is no users for DUMP_WRITE macro in
> fs/binfmt_elf.c. Thus remove the definition of DUMP_WRITE macro.
>
> Signed-off-by: Daisuke HATAYAMA <[email protected]>


I assume you at least did a build test?

I like this patch:

Acked-by: WANG Cong <[email protected]>

> ---
>  fs/binfmt_elf.c |   17 ++++++++++-------
>  1 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 5f7be51..9666a8a 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -1277,10 +1277,6 @@ static int writenote(struct memelfnote *men, struct file *file,
>  }
>  #undef DUMP_WRITE
>
> -#define DUMP_WRITE(addr, nr)   \
> -       if ((size += (nr)) > limit || !dump_write(file, (addr), (nr))) \
> -               goto end_coredump;
> -
>  static void fill_elf_header(struct elfhdr *elf, int segs,
>                            u16 machine, u32 flags, u8 osabi)
>  {
> @@ -1981,7 +1977,10 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un
>        fs = get_fs();
>        set_fs(KERNEL_DS);
>
> -       DUMP_WRITE(elf, sizeof(*elf));
> +       size += sizeof(*elf);
> +       if (size > limit || !dump_write(file, elf, sizeof(*elf)))
> +               goto end_coredump;
> +
>        offset += sizeof(*elf);                         /* Elf header */
>        offset += (segs + 1) * sizeof(struct elf_phdr); /* Program headers */
>        foffset = offset;
> @@ -1995,7 +1994,9 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un
>
>                fill_elf_note_phdr(&phdr, sz, offset);
>                offset += sz;
> -               DUMP_WRITE(&phdr, sizeof(phdr));
> +               size += sizeof(phdr);
> +               if (size > limit || !dump_write(file, &phdr, sizeof(phdr)))
> +                       goto end_coredump;
>        }
>
>        dataoff = offset = roundup(offset, ELF_EXEC_PAGESIZE);
> @@ -2026,7 +2027,9 @@ static int elf_core_dump(long signr, struct pt_regs *regs, struct file *file, un
>                        phdr.p_flags |= PF_X;
>                phdr.p_align = ELF_EXEC_PAGESIZE;
>
> -               DUMP_WRITE(&phdr, sizeof(phdr));
> +               size += sizeof(phdr);
> +               if (size > limit || !dump_write(file, &phdr, sizeof(phdr)))
> +                       goto end_coredump;
>        }
>
>        if (!elf_core_write_extra_phdrs(file, offset, &size, limit))
> --
> 1.6.5.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

2009-12-16 02:24:30

by Hatayama, Daisuke

[permalink] [raw]
Subject: Re: [RFC, PATCH 2/4] elf_core_dump(): Remove DUMP_WRITE macro


> On Tue, Dec 15, 2009 at 10:41 AM, Daisuke HATAYAMA
> <[email protected]> wrote:
> > In DUMP_WRITE macro, some local variables and goto statement are used
> > implicitly, which makes it confusing to understand how the dumping
> > process behaves. So, clean up it by unfolding the macro body directly
> > to the point where the original macro is used plus a bit of style
> > change.
> >
> > Now that the previous patch replaced ELF_CORE_EXTRA_* macros in
> > fs/binfmt_elf.c, there is no users for DUMP_WRITE macro in
> > fs/binfmt_elf.c. Thus remove the definition of DUMP_WRITE macro.
> >
> > Signed-off-by: Daisuke HATAYAMA <[email protected]>
>
>
> I assume you at least did a build test?

Oh..sorry. I have forgotten writting about test.

For IA64, i386 and UML-i386, I tested on the real machine.
For s390, arm and xtensa, I only did a build test.

> I like this patch:

Thanks.