Messages in this thread | | | Date | Tue, 9 Nov 2010 18:56:17 +0100 | From | Wolfram Sang <> | Subject | Re: [PATCH 1/2] Introduce fat_msg() for unified kernel messages |
| |
Hi Joe,
On Tue, Nov 09, 2010 at 09:25:41AM -0800, Joe Perches wrote: > On Tue, 2010-11-09 at 17:55 +0100, Alexey Fisher wrote: > > Add fat_msg() function to unify printkas. And use it > > to report mounts and remounts. > > Hi Alexey. > > > new dmesg looks like this: > > [ 6264.957109] FAT-fs (sdg1): Mounted. Opts: uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush > > [ 6402.175028] FAT-fs (sdg1): re-mounted. Opts: (null) > > > > Signed-off-by: Alexey Fisher <bug-track@fisher-privat.net> > > --- > > fs/fat/inode.c | 20 ++++++++++++++++++++ > > 1 files changed, 20 insertions(+), 0 deletions(-) > > > > diff --git a/fs/fat/inode.c b/fs/fat/inode.c > > index ad6998a..4699173 100644 > > --- a/fs/fat/inode.c > > +++ b/fs/fat/inode.c > > @@ -37,6 +37,20 @@ > > static int fat_default_codepage = CONFIG_FAT_DEFAULT_CODEPAGE; > > static char fat_default_iocharset[] = CONFIG_FAT_DEFAULT_IOCHARSET; > > > > +/* unify messages. > > + * this function is copy of ext4_msg() from fs/ext4/super.c > > + */ > > +void fat_msg(struct super_block *sb, const char *prefix, > > + const char *fmt, ...) > > +{ > > + va_list args; > > + > > + va_start(args, fmt); > > + printk("%sFAT-fs (%s): ", prefix, sb->s_id); > > + vprintk(fmt, args); > > + printk("\n"); > > + va_end(args); > > +} > > A few comments: > > The first patch should put the prototype for fat_msg in > fs/fat/fat.h file and the body as done here. > > The prototype should use attribute_ printf so format > and argument are type checked. > > This would be better using the %pV extension so a > single printk call is done and the dmesg log can not be > interleaved. > > prefix is a bit misleading, perhaps level is better.
Note that this routine was almost 1:1 taken from ext2, it seems (which is a sensible approach), so these issues are present more than once.
Regards,
Wolfram
-- Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |