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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ