Re: [PATCH] support of hfsx ( case comparaison )

[Pavel Roskin <proski@gnu.org>]

Hello!

I think the ChangeLog needs to be improved.  It's immodest to claim
"complete" support for something.  It's a very strong statement.  It's
better to say "improve".  Or better yet, let's be specific.  Also please
spell check the entry.  "insensitive" and "insentive" is not the same.
You want the former.   Please end sentences with a period.  I would
write the ChangeLog entry as:

	* fs/hfsplus.c: Use case sensitive comparison for hfsx when
	required by the filesystem settings.

Actually, it would be better to list the functions involved.  I just
want to show how long descriptions can be improved.

I also prefer not to use any negative logic, and it's easier to get it
wrong.  Humans are notoriously bad at logic.  Let's have
grub_hfsplus_is_case_sensitive().  I would write it like this:

static inline int
grub_hfsplus_is_case_sensitive (struct grub_hfsplus_data *data)
{
  if ((grub_be_to_cpu16 (data->volheader.magic) == GRUB_HFSPLUSX_MAGIC)
      && (data->catalog_cmp_key == GRUB_HFSPLUSX_BINARYCOMPARE))
    return 1;
  else
    return 0;
}

No need to handle unknown magic numbers.

We may need to use a comparison table as in hfs.c, as least for the
first 256 Unicode characters, but it's a separate issue.

-- 
Regards,
Pavel Roskin