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

[Michael Scherer]

Le mercredi 03 juin 2009 à 17:23 -0400, Pavel Roskin a écrit :
> 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.

Hi, 

Sorry, I am far from being a native english speaker, so that's why some
badly translated expression slipped in and I didn't paid much attention
to the changelog, I was more concerned by the code formating.

> 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.

Ok, I have taken in account your remark. Here is another patch.


> 
> 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.

That a little bit too complex for me, I have just patched grub for the
simplest case, and for the issue I faced on my own computer. 

-- 
Michael Scherer

grub-hfsplus.diff
Description: Text Data