------------------------------ On Tue, Apr 8, 2014 5:15 PM BST Vyacheslav Dubeyko wrote: >On Tue, 2014-04-08 at 16:06 +0100, Anton Altaparmakov wrote: >> Hi Hin-Tak, >> >> That now looks good. >> >> I agree with you that doing a major cleanup of how attribute names are >> handled is a separate patch. As you say that is much more work and it >> is not as trivial as this. I think doing the attribute handling >> properly requires reworking how it all fits together because it is >> imposing the name length checks at the wrong level - they need to be >> done after conversion to UTF-16 not whilst they are still in the >> current NLS (except of course you want to check that they are not too >> big to fit in the buffer though it is not at all clear to me why the >> argument is being copied into a buffer in the first place at least in >> one of the places I looked at - seems a waste of CPU time to me). >> >Yes, it is possible to split the work on several patches. I agree with >it. > >But I suppose that these patches needs to be joined in one patchset. >Because reported issue can be solved by fix as for file names as for >xattr names. > >Of course, it is possible to fix the issue with file names, firstly, and >then the issue with xattr names. But it doesn't make sense to mix fixes >for file names and for xattr names in one patch. Because partial fix in >hfsplus_listxattr() doesn't fix the issue for xattr names. > Think of it as "fixes worst-case usage of hfsplus_uni2asc()", then it makes sense to have this as one patch. Allowing full usage of getting and setting of long multi-lingual attributes, and checking other uses of HFSPLUS_ATTR_MAX_STRLEN really need to be another (multiple others). There are only two usages of hfsplus_uni2asc(), one for names and one for attributes. >> Also, I don't think it is needed to set up a kmem cache for this - >> kmalloc is perfect - as at any one time there will be a vanishingly >> small number of these buffers allocated at any one time. After all, >> how many concurrent readdir() calls are there going to be on a volume >> at any one time? It is not like the struct inode or other long lived >> memory structures where you end up with thousands of them in memory at >> any one time... > >Yes, kmalloc can be enough for readdir() case. But I suppose that it >needs to use kmem cache for the case of xattr names. It will be more >efficient solution. > -- 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
