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

On Wed, 2014-04-09 at 09:58 +0100, Anton Altaparmakov wrote:

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

Yes, right things should be right. :)

I misunderstand where you've found in my sentence differences with your
statement "to allow anything that would work on OSX". So, I see only
useless objection here. Sorry.

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

Yes, it's unbelievable that another guy has another opinion. :) I have
another vision here. Sorry. But I have right to think in other way and
to share my opinion freely. And I don't think that your vision is single
and only way.

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

I see similarity between xattr name buffers and inodes buffers. So, I've
suggested to use kmem cache because of such similarity. As far as I can
judge, you mean that xattr name buffers will live short time. And, as a
result, kmalloc will be much better choice. Do you mean this? Am I
correct? Otherwise, it makes sense to use simple kmalloc and for inode
buffers.

Thanks,
Vyacheslav Dubeyko.


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