[Devel] Re: [PATCH 1/1] Syslog are now containerized

[Matt Helsley]

Thanks for working on this. Some minor style issues below:

On Thu, Feb 11, 2010 at 01:00:20AM -0500, Jean-Marc Pigeon wrote:
> 	Added syslog.c such container /proc/kmsg and host /proc/kmsg
> 	do not leak in each other.
> 	Running rsyslog daemon within a container won't destroy
> 	host kernel messages.
> ---

<snip>

> diff --git a/kernel/syslog.c b/kernel/syslog.c
> new file mode 100644
> index 0000000..69d30a9
> --- /dev/null
> +++ b/kernel/syslog.c
> @@ -0,0 +1,157 @@

<snip>

> +		(void) kfree(old_buf);
> +		}
> +	(void) printk(KERN_NOTICE "log_buf_len: %u\n", syslog_ns->buf_len);

I don't understand why you chose to put in all of the "(void)" casts.
They do nothing -- they don't change how the code works nor do they
improve the readability of the code.

> +	return syslog_ns;
> +}
> +/*
> + * Procedure to free all ressources tied to syslog
> + *
> + */
> +struct syslog_ns *syslog_free(struct syslog_ns *syslog)
> +
> +{
> +	if (syslog != (struct syslog_ns *)0) {

Why aren't you using NULL? Better yet, in these cases it's common to use:

if (syslog) {

Once this gets out of the RFC steps perhaps run these through checkpatch..

Cheers,
	-Matt Helsley
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers