Re: [PATCH] fs/ntfs3: Fix logic in ntfs_cmp_names_cpu

[Mark Harmstone <mark@harmstone.com>]

On 28/9/21 23:07, Kari Argillander wrote:
 > So I have checked this now. Yes function will always return non-zero
 > value if we use bothcase and this is intended behavier. But if
 > bothcase is false and upcase is not NULL then function can return zero
 > value.

Thanks Kari. It's not clear to the casual observer what both_case even
means... It's only ever true when called from cmp_fnames for a non-DOS
$FILE_NAME. Given that DOS names are guaranteed to be uppercase, I'm
guessing it's supposed to mean "no need to uppercase this string as
well", but that's not what it's doing.

 > Do you have some problems? There could be that in some place this or
 > other cmp function is called with wrong parameters. Example with
 > bothcase true when only case_insentive is needed.

Yes, it forces case-sensitivity - i.e. `ls WINDOWS` give ENOENT, whereas
`ls Windows` succeeds. cmp_fnames <- indx_find <- dir_search_u <-
ntfs_lookup.

I think perhaps ntfs_cmp_names_cpu was supposed to look something like
the following?

int ntfs_cmp_names_cpu(const struct cpu_str *uni1, const struct le_str *uni2,
		       const u16 *upcase, bool uni2_not_uc)
{
	const u16 *s1 = uni1->name;
	const __le16 *s2 = uni2->name;
	size_t l1 = uni1->len;
	size_t l2 = uni2->len;
	size_t len = min(l1, l2);

	while (len > 0) {
		int diff = *s1 - le16_to_cpu(*s2);
		
		if (diff) {
			if (upcase)
				break;
			
			return diff;
		}

		s1++;
		s2++;
		len--;
	}

	if (len > 0 && upcase) {
		while (len > 0) {
			int diff;
			u16 c = le16_to_cpu(*s2);

			if (uni2_not_uc)
				c = upcase_unicode_char(upcase, c);

			diff = upcase_unicode_char(upcase, *s1) - c;

			if (diff)
				return diff;

			s1++;
			s2++;
			len--;
		}
	}

	return l1 - l2;
}

Mark