linux-kernel - Re: [PATCH 1/2] Moved core dump functionality into its own file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120804064304.GA5195@gmail.com>
Date:	Sat, 4 Aug 2012 08:43:04 +0200
From:	Ingo Molnar <mingo@...nel.org>
To:	Alex Kelly <eshink@...il.com>
Cc:	Alexander Viro <viro@...iv.linux.org.uk>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Alex Kelly <alex.page.kelly@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/2] Moved core dump functionality into its own file


* Alex Kelly <eshink@...il.com> wrote:

> From: Alex Kelly <alex.page.kelly@...il.com>
> 
> This was done in preparation for making core dump functionality optional.

Please use present tense and a sane title, something like:

  fs: Move core dump functionality into its own file
  fs: Make core dump functionality optional

While looking at that code, there's also a few uglies in it, 
like:

> +	return nr;
> +}
> +
> +
> +/*

> +
> +	/* Repeat as long as we have more pattern to process and more output
> +	   space */

> +}
> +
> +
> +void do_coredump(long signr, int exit_code, struct pt_regs *regs)

> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -413,6 +413,7 @@ static inline void arch_pick_mmap_layout(struct mm_struct *mm) {}
>  
>  extern void set_dumpable(struct mm_struct *mm, int value);
>  extern int get_dumpable(struct mm_struct *mm);
> +extern int __get_dumpable(unsigned long mm_flags);

These prototypes should move out of sched.h and into a 
coredump.h header or so.

If we are touching this code lets use the opportunity and do 
this right.

Note, I'd suggest to put any such further changes into separate, 
additional patches at the end: a cleanup patch, a header file 
introduction patch, etc. - and keep the "dumb code movement" 
chnage in the initial patch. This will make it all much easier 
to review.

Please also review the code for more uglies, the above was just 
a very quick stylistic scan.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/