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.
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.
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.
I admit that this was not clenest way of implementing this. Maybe we
should make bothcase enum with three values. Example CASE_SENSITIVE,
CASE_INSENTIVE, CASE_BOTH. I do not like name BOTH so if anyone know
any better please tell.
Argillander
>
> 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
>
>
