Re: [PATCH V2] hfsplus: fixes worst-case unicode to char conversion of file names and attributes

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

 



Hi Vyacheslav,

On 9 Apr 2014, at 07:46, Vyacheslav Dubeyko <slava@xxxxxxxxxxx> wrote:
> On Tue, 2014-04-08 at 20:53 +0100, Hin-Tak Leung wrote:
> [snip]
>> 
>> If you insist on doing Chinese all the way, 127/3 = 42 is really a bit low for a
>> descriptive string of any usage, but does anybody use non-english extended
>> attributes on any OSes?
>> 
> 
> If you think that nobody use non-english extended attributes then it
> doesn't make sense to make any fixes in xattr subsystem of HFS+.

It doesn't matter what anyone things.  It matters to do what is right.  And right is to allow anything that would work on OSX.

> And partial code changing in hfsplus_listxattr() is completely useless.

No it is not at all useless.  It is perfectly valid to run listxattr() without then running getxattr() on all the attributes!  If an application cares about attributes foo and bar and there is also an attribute with a super long name then as things are at the moment, the listxattr() will fail with EIO error and the application will fail badly probably telling you your file system is corrupt.  With the fix from Hin-Tak, the listxattr() will work fine and then the application will work fine because it will only getxattr() foo and bar.  The difference being an application failing due to apparently corrupt file system or broken disk and an application working.  How can you possibly state this fix is useless!?!  Unbelievable...

> Because getxattr() and setxattr() series of methods will be the primary
> source of error in the case of worst-case unicode to char conversion.
> 
> Otherwise, you can use any splitting of fixes between patches. In my
> last e-mail I suggested to use kmem cache approach for xattr names
> instead of kmalloc only.

Yes but I don't think you have given a good reason why it would be worth to create a kmem cache for such temporary buffers and IMO it makes no sense to use them - kmalloc and friends are a better choice in this use case.  Please remember that kmalloc is effectively using a kmem cache - just a set of generic ones (depending on size of allocation - just look at /proc/slabinfo some time) instead of a specialised one so there is no benefit in using a dedicated kmem cache unless you are going to have lots of those buffers around which you are not as they only live for the duration of the get/set/list function call.

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge
J.J. Thomson Avenue, Cambridge, CB3 0RB, UK

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