Re: [PATCH] hfsplus: fix FS driver name in printks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



--- On Tue, 29/1/13, Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:

> On Tue, 2013-01-29 at 09:22 +0000,
> Mike Fleetwood wrote:
> > On Tue, Jan 29, 2013 at 09:49:25AM +0400, Vyacheslav
> Dubeyko wrote:
> > > On Mon, 2013-01-28 at 20:23 +0000, Mike Fleetwood
> wrote:
> > > > Correct the name of the hfsplus FS driver as
> used in printk calls.
> > > > "hfs:" -> "hfsplus:".
> > > > 
> > > > Signed-off-by: Mike Fleetwood <mike.fleetwood@xxxxxxxxxxxxxx>
> > > > ---
> > > > 
> > > > Hi,
> > > > 
> > > > Is there a current reason why the hfsplus FS
> driver uses "hfs:" almost
> > > > exclusively rather than "hfsplus:" as its
> name in printk calls?
> > > 
> > > There are as minimum two reason for leaving "hfs:"
> prefix in peace: (1)
> > > historical - it is like code style of "old"
> library; (2) the prefix
> > > "hfs:" is shorter - so, it gives opportunity to
> make more descriptive
> > > comments by means of one line under 80 symbols
> kernel code style
> > > requirement.

I am inclined to side on the side of keep it short and as-is. Note also that HFS+ is more or less a technical name on technical circles. Apple's more public layman oriented literature refers to the new format as "HFS extended" or some such, and talked abou the old one as 'HFS Standard'. In any case, there is not much chance for confusion, for when the message is important, and where it matters - I don't think current Mac OS X is even capable of formatting disks as the HFS standard.

Just as a head up and a vaguely related matter: I have also a patch sitting in my pile which converts the optionally-compiled in warnings/messages as dynamic debugging messages. That's a useful change since some transient error condition might be gone if one need to re-compile a new kernel with hfs related debugging switched on, but with conversion to dynamic debugging there is a better chance of obtaining more information when the error condition is still observable.

> > > By the way, did you check your patch by
> scripts/checkpatch.pl script?
> > > 
> > > Moreover, there are hfsplus driver's patches in
> linux-next that uses
> > > "hfs:" prefix.
> > > 
> > > I doubt that this patch can improve hfsplus driver
> quality. It looks
> > > like changes in many places without changing
> anything in essence.
> > > 
> > > With the best regards,
> > > Vyacheslav Dubeyko.  
> > > 
> > 
> > In terms of line length I was applying the exception in
> the CodingStyle
> > which says "never break user-visible strings such as
> printk messages,
> > because that breaks the ability to grep for them." to
> allow lines be
> > longer than 80 characters.
> > 
> > I did use checkpatch.pl.  It reported this for
> every printk:
> >     WARNING: Prefer
> netdev_err(netdev, ... then dev_err(dev, ... then
> pr_err(...  to printk(KERN_ERR ...
> 
> So, if we begin to modify error messages in the hfsplus
> driver then,
> maybe, it makes sense to exchange the printk() on pr_err()
> and to
> prepare define for "hfsplus:" prefix? Then, we make
> checkpatch.pl
> completely happy.
> 
> > After seeing that ext2/3/4, btrfs and xfs use printk
> and not any of
> > those functions I followed the majority.  It also
> reported a couple of:
> >     WARNING: line over 80
> characters
> > I was applying the above exception.
> > 
> > Absoutely this patch doesn't fix any faults and is not
> for 3.8.0-rc* but
> > for linux next.  I just though that it would be
> useful for users to be
> > told the name of the FS driver generating the message
> rather than a
> > different one.
> > 
> 
> Anyway, I think that changing "hfs:" on "hfsplus:" is not
> important.
> 
> > Would an equlivant patch be accepted for linux-next?
> > How do I send it to linux-next?
> > 
> 
> I mean
> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> when I am talking about linux-next. So, you can prepare
> patches are
> based on this repository.
> 
> With the best regards,
> Vyacheslav Dubeyko.
> 
> > Thank you Vyacheslav for reviewing my patch,
> > Mike
> > 
> > > 
> > > > Assuming not here's a patch to fix.
> > > > 
> > > > (Any code which may have been copied between
> hfs and hfsplus has since
> > > > diverged significantly).
> > > > 
> > > > Thanks,
> > > > Mike
> > > > 
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux