On Tue, Sep 28, 2021 at 07:33:00PM +0100, Mark Harmstone wrote: > I'm not quite sure what was intended by the logic in ntfs_cmp_names_cpu, > but it was broken when comparing case-insensitively. If the strings > matched but differ in case, diff1 would be non-zero, meaning that the > function would always return a non-zero value. Original idea can be found in here [1]. So recap. If we need first check lower case and then upcase we can actually do this in just one time. We first find first mismatch. Then we in that point on try to find if we find another mismatch when upcasing. This way only one compare round is needed in worst case. I will look this patch more closly soon. [1]: https://lore.kernel.org/all/20210103231755.bcmyalz3maq4ama2@kari-VirtualBox/ > > Signed-off-by: Mark Harmstone <mark@xxxxxxxxxxxxx> > --- > fs/ntfs3/upcase.c | 18 ++++++++---------- > 1 file changed, 8 insertions(+), 10 deletions(-) > > diff --git a/fs/ntfs3/upcase.c b/fs/ntfs3/upcase.c > index b5e8256fd710..dd28c2133a4c 100644 > --- a/fs/ntfs3/upcase.c > +++ b/fs/ntfs3/upcase.c > @@ -74,31 +74,29 @@ int ntfs_cmp_names_cpu(const struct cpu_str *uni1, const struct le_str *uni2, > size_t l1 = uni1->len; > size_t l2 = uni2->len; > size_t len = min(l1, l2); > - int diff1 = 0; > - int diff2; > + int diff; > > if (!bothcase && upcase) > goto case_insentive; > > for (; len; s1++, s2++, len--) { > - diff1 = *s1 - le16_to_cpu(*s2); > - if (diff1) { > + diff = *s1 - le16_to_cpu(*s2); > + if (diff) { > if (bothcase && upcase) > goto case_insentive; > > - return diff1; > + return diff; > } > } > return l1 - l2; > > case_insentive: > for (; len; s1++, s2++, len--) { > - diff2 = upcase_unicode_char(upcase, *s1) - > + diff = upcase_unicode_char(upcase, *s1) - > upcase_unicode_char(upcase, le16_to_cpu(*s2)); > - if (diff2) > - return diff2; > + if (diff) > + return diff; > } > > - diff2 = l1 - l2; > - return diff2 ? diff2 : diff1; > + return l1 - l2; > } > -- > 2.31.1 > >
