------------------------------
On Wed, Apr 23, 2014 2:59 AM BST Andrew Morton wrote:
>On Wed, 23 Apr 2014 02:43:00 +0100 (BST) Hin-Tak Leung <htl10@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
>> >+++ a/fs/hfsplus/xattr.c
>> >@@ -726,6 +726,7 @@ ssize_t hfsplus_listxattr(struct dentry
>> >
>> > end_listxattr:
>> > ______ kfree(strbuf);
>> >+out:
>> > ______ hfs_find_exit(&fd);
>> > ______ return res;
>> > }
>> >
>> >But obviously the patches you sent were not the patches you tested.
>> >What happened here?
>> >
>>
>> Hi Andrew,
>>
>> Am terribly sorry - my fault of using/working/testing under one kernel (3.13.9),
>> then cherry-picking onto Linus' HEAD and collapsing there. A couple of small
>> changes got dropped/messed up. There is the missing headers
>> (#include <linux/nls.h>) in xattr*.c you spotted in the other mail; the above
>> is meant to be corrected by re-using the label nearby (as kfree(null) is okay...), rather than adding
>> a new label.
>>
>> diff --git a/fs/hfsplus/xattr.c b/fs/hfsplus/xattr.c
>> index 3ddf50c..2a38647 100644
>> --- a/fs/hfsplus/xattr.c
>> +++ b/fs/hfsplus/xattr.c
>> @@ -670,7 +670,7 @@ ssize_t hfsplus_listxattr(struct dentry *dentry, char *buffer, size_t size)
>> XATTR_MAC_OSX_PREFIX_LEN + 1, GFP_KERNEL);
>> if (!strbuf) {
>> res = -ENOMEM;
>> - goto out;
>> + goto end_listxattr;
>> }
>>
>> err = hfsplus_find_attr(inode->i_sb, inode->i_ino, NULL, &fd);
>>
>> Do you want me to re-submit? I checked the diff of the two branches, it
>> is just these two issues.
>>
>
>Is OK - what I have now saves 30 cycles on a path which never happens ;)
>
Two mistakes (and both lead to failed compilation... ) is kind of unforgivable :-(.
The label mistake was simply because I had it your way, then just edit out the new label... The missing header was because it was the first of a bunch, and
git cherry-pick patch1..patchN
misses the first. I won't make the same mistake again - just possibly making new ones :-).
>It would help if you could grab tomorrow's linux-next and retest, to
>check that everything landed OK.
>
will do.
>
>How were you testing this anyway? It's a pretty intricate patchset and
>presumably some effort went into developing the testcases. Perhaps you
>have something which we can immortalize in tools/testing/selftests/?
It came able because I was staring at hfsplus_readir() to try to see how it gets confused just doing 'du' on a large directory. (It is a reproduceable problem, no solution is found yet, although it isn't likely related as others see to have similar issues of theirs doing updatedb or rm -rf)
So I see the disk to user encoding of file name was wrong - and I do have some non-english file names aleady there, though I made some new ones with just 'touch ... '. Then Vyacheslav mentions the attributes need similar changes. So the attribute testcase was made up from scratch; I thought it would be nice to include the details for people who don't read/write chinese hence the rather extended message.
As for immortalise it, we could just do 'touch ... ' then remove the same thing, and make sure there is no failure with either touch or removal. I think we could adapt the long file name test from Sougata and extend it a bit? The limit to file names and attribute names is fs dependent though. (and also locale dependent, for fs which has a definite encoding within like hfs+)
btw, the linux kernel seems to have inherited a generic limitation to attribute *values* being non-nulls, from xfs/sgi. Mac OS X attribute values can have nulls - in fact their set/get attribute tool, xattr, routinely do hex dumps, rather than print as string, for this reason, I think.
Hin-Tak
--
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
