linux-kernel - Re: [PATCH 1/3] Unionfs: use printk KERN

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 for Android: free password hash cracker in your pocket

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

Message-Id: <1199341581.6615.39.camel@localhost>
Date:	Wed, 02 Jan 2008 22:26:20 -0800
From:	Joe Perches <joe@...ches.com>
To:	Erez Zadok <ezk@...sunysb.edu>
Cc:	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, viro@....linux.org.uk,
	hch@...radead.org
Subject: Re: [PATCH 1/3] Unionfs: use printk KERN_CONT for debugging
	messages

On Thu, 2008-01-03 at 00:57 -0500, Erez Zadok wrote:
> diff --git a/fs/unionfs/debug.c b/fs/unionfs/debug.c
> index c2b8b58..5f1d887 100644
> --- a/fs/unionfs/debug.c
> +++ b/fs/unionfs/debug.c
>  void __show_inode_times(const struct inode *inode,
> @@ -472,15 +473,15 @@ void __show_inode_times(const struct inode *inode,
>  		if (unlikely(!lower_inode))
>  			continue;
>  		pr_debug("IT(%lu:%d): ", inode->i_ino, bindex);
> -		pr_debug("%s:%s:%d ", file, fxn, line);
> -		pr_debug("um=%lu/%lu lm=%lu/%lu ",
> -			 inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> -			 lower_inode->i_mtime.tv_sec,
> -			 lower_inode->i_mtime.tv_nsec);
> -		pr_debug("uc=%lu/%lu lc=%lu/%lu\n",
> -			 inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> -			 lower_inode->i_ctime.tv_sec,
> -			 lower_inode->i_ctime.tv_nsec);
> +		printk(KERN_CONT "%s:%s:%d ", file, fxn, line);
> +		printk(KERN_CONT "um=%lu/%lu lm=%lu/%lu ",
> +		       inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> +		       lower_inode->i_mtime.tv_sec,
> +		       lower_inode->i_mtime.tv_nsec);
> +		printk(KERN_CONT "uc=%lu/%lu lc=%lu/%lu\n",
> +		       inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> +		       lower_inode->i_ctime.tv_sec,
> +		       lower_inode->i_ctime.tv_nsec);
>  	}
>  }
>  

I think printks should be single statements and
KERN_CONT should be used as sparingly as possible.

Perhaps:
		pr_debug("IT(%lu:%d): %s:%s:%d "
			 "um=%lu/%lu lm=%lu/%lu "
			 "uc=%lu/%lu lc=%lu/%lu\n",
			 inode->i_ino, bindex, file, fnx, line,
			 inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
			 lower_inode->i_mtime.tv_sec,
			 lower_inode->i_mtime.tv_nsec
			 inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
			 lower_inode->i_ctime.tv_sec,
			 lower_inode->i_ctime.tv_nsec);

> @@ -497,15 +498,15 @@ void __show_dinode_times(const struct dentry *dentry,
>  			continue;
>  		pr_debug("DT(%s:%lu:%d): ", dentry->d_name.name, inode->i_ino,
>  			 bindex);
> -		pr_debug("%s:%s:%d ", file, fxn, line);
> -		pr_debug("um=%lu/%lu lm=%lu/%lu ",
> -			 inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> -			 lower_inode->i_mtime.tv_sec,
> -			 lower_inode->i_mtime.tv_nsec);
> -		pr_debug("uc=%lu/%lu lc=%lu/%lu\n",
> -			 inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> -			 lower_inode->i_ctime.tv_sec,
> -			 lower_inode->i_ctime.tv_nsec);
> +		printk(KERN_CONT "%s:%s:%d ", file, fxn, line);
> +		printk(KERN_CONT "um=%lu/%lu lm=%lu/%lu ",
> +		       inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
> +		       lower_inode->i_mtime.tv_sec,
> +		       lower_inode->i_mtime.tv_nsec);
> +		printk(KERN_CONT "uc=%lu/%lu lc=%lu/%lu\n",
> +		       inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
> +		       lower_inode->i_ctime.tv_sec,
> +		       lower_inode->i_ctime.tv_nsec);
>  	}
>  }
>  

and
		pr_debug("DT(%s:%lu:%d): %s:%s:%d "
			 "um=%lu/%lu lm=%lu/%lu "
			 "uc=%lu/%lu lc=%lu/%lu\n",
			 dentry->d_name.name, inode->i_ino, bindex,
			 file, fnx, line,
			 inode->i_mtime.tv_sec, inode->i_mtime.tv_nsec,
			 lower_inode->i_mtime.tv_sec,
			 lower_inode->i_mtime.tv_nsec
			 inode->i_ctime.tv_sec, inode->i_ctime.tv_nsec,
			 lower_inode->i_ctime.tv_sec,
			 lower_inode->i_ctime.tv_nsec);

> 
> @@ -524,9 +525,10 @@ void __show_inode_counts(const struct inode *inode,
>  		lower_inode = unionfs_lower_inode_idx(inode, bindex);
>  		if (unlikely(!lower_inode))
>  			continue;
> -		pr_debug("SIC(%lu:%d:%d): ", inode->i_ino, bindex,
> -			 atomic_read(&(inode)->i_count));
> -		pr_debug("lc=%d ", atomic_read(&(lower_inode)->i_count));
> -		pr_debug("%s:%s:%d\n", file, fxn, line);
> +		printk(KERN_CONT "SIC(%lu:%d:%d): ", inode->i_ino, bindex,
> +		       atomic_read(&(inode)->i_count));
> +		printk(KERN_CONT "lc=%d ",
> +		       atomic_read(&(lower_inode)->i_count));
> +		printk(KERN_CONT "%s:%s:%d\n", file, fxn, line);
>  	}
>  }

and
		pr_debug("SIC(%lu:%d:%d): lc=%d %s:%s:%d\n",
			 inode->i_ino, bindex,
			 atomic_read(&(inode)->i_count),
			 atomic_read(&(lower_inode)->i_count),
			 file, fxn, line);

--
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/