hfsplus: fix the bug that cannot recognize files with hangul file name

The unicode of hangul from macOS is decomposed. There has a bug that mistake decomposed unicode for composed when change unicode to ascii, so it cannot recognize the hangul correctly.

Signed-off-by Ting-Chang Hou <[email protected]>

---
 fs/hfsplus/unicode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/hfsplus/unicode.c +0 -0

index dfa90c2..2daf7b0 100644

fs/hfsplus/unicode.c +0 -0

 	while (ustrlen > 0) {
 		c0 = be16_to_cpu(*ip++);

Signature · 1 line

2.7.4

Could you please share the examples of incorrect and correct behaviour?

What is it exactly that you are trying to fix? You mention an issue with hangul characters, but I failed to trigger it. Could you expand on that?

Quoted 14 lines

The unicode of hangul from macOS is decomposed. There has a bug that mistake decomposed unicode for composed when change unicode to ascii, so it cannot recognize the hangul correctly.

Signed-off-by Ting-Chang Hou <[email protected]>

---
 fs/hfsplus/unicode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

fs/hfsplus/unicode.c +0 -0

index dfa90c2..2daf7b0 100644

fs/hfsplus/unicode.c int hfsplus_uni2asc(struct super_block *sb, · line 138

Quoted 5 lines

fs/hfsplus/unicode.c +1 -1

@@ -135,7 +135,7 @@ int hfsplus_uni2asc(struct super_block *sb,
 	ustrlen = be16_to_cpu(ustr->length);
 	len = *len_p;
 	ce1 = NULL;
-	compose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
+	compose = test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);

I'm not sure this is a mistake. The developers probably wanted the filenames to be recomposed before being presented in utf8. With your patch, if you try the following (with the default mount options):

touch Á
ls | hexdump -C

the utf8 output filename will be using the combining accent (CC 81) instead of the Á character (C3 81). This is a bit annoying because it won't print correctly in my terminal anymore.

Quoted 10 lines

Could you please share the examples of incorrect and correct behaviour?

What is it exactly that you are trying to fix? You mention an issue
with
hangul characters, but I failed to trigger it. Could you expand on that?

Hi all,
There is an example.I use Mac mini to format my usb disk to HFS+ and touch
the file with filename "공유"(a Korean actor, https://goo.gl/VcBsrn)
on
it.
After it, I mount the usb disk on my ubuntu(Linux 4.14.0+) and get the
following error message when trying to ls and cp the file:

ls: cannot access 공유: No such file or directory
cp: cannot stat ‘공유’: No such file or directory

It seem's a problem for a long time(https://goo.gl/LiWGe5). After applying my patch, I can ls and cp the file correctly.

Quoted 35 lines · Ting-Chang Hou

Quoted 17 lines

The unicode of hangul from macOS is decomposed. There has a bug that
mistake decomposed unicode for composed when change unicode to ascii,
so it cannot recognize the hangul correctly.

Signed-off-by Ting-Chang Hou <[email protected]>

---
 fs/hfsplus/unicode.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

fs/hfsplus/unicode.c +0 -0

index dfa90c2..2daf7b0 100644

fs/hfsplus/unicode.c int hfsplus_uni2asc(struct super_block *sb, · line 140

Quoted 7 lines

fs/hfsplus/unicode.c +1 -1

@@ -135,7 +135,7 @@ int hfsplus_uni2asc(struct super_block *sb,
  	ustrlen = be16_to_cpu(ustr->length);
  	len = *len_p;
  	ce1 = NULL;
-	compose = !test_bit(HFSPLUS_SB_NODECOMPOSE,
 &HFSPLUS_SB(sb)->flags);
+	compose = test_bit(HFSPLUS_SB_NODECOMPOSE,
 &HFSPLUS_SB(sb)->flags);

I'm not sure this is a mistake. The developers probably wanted the filenames to be recomposed before being presented in utf8. With your
patch,
if you try the following (with the default mount options):

  touch Á
  ls | hexdump -C

Quoted 7 lines

the utf8 output filename will be using the combining accent (CC
81)
instead
of the Á character (C3 81). This is a bit annoying because it won't
print
correctly in my terminal anymore.

I'm not exatly know why combining accent cannot print correctly in terminal
and how to avoid it. Whether apply my patch or not, my terminal cannot
print the hangul charactor correctly.

Quoted 10 lines

Quoted 10 lines

Could you please share the examples of incorrect and correct behaviour?

What is it exactly that you are trying to fix? You mention an issue
with
hangul characters, but I failed to trigger it. Could you expand on that?

Quoted 10 lines

Hi all,
There is an example.I use Mac mini to format my usb disk to HFS+ and touch
the file with filename "공유"(a Korean actor, https://goo.gl/VcBsrn)
on
it.
After it, I mount the usb disk on my ubuntu(Linux 4.14.0+) and get the
following error message when trying to ls and cp the file:

Could you share the all mount options that you used? It looks that you simply don't use the proper mount options set under the Linux. Because MacOS X will mount properly always.

Thanks,
Vyacheslav Dubeyko.

Quoted 57 lines

ls: cannot access 공유: No such file or directory
cp: cannot stat ‘공유’: No such file or directory

It seem's a problem for a long time(https://goo.gl/LiWGe5). After applying my patch, I can ls and cp the file correctly.

Quoted 35 lines · Ting-Chang Hou

Quoted 17 lines

The unicode of hangul from macOS is decomposed. There has a bug that
mistake decomposed unicode for composed when change unicode to ascii,
so it cannot recognize the hangul correctly.

Signed-off-by Ting-Chang Hou <[email protected]>

---
 fs/hfsplus/unicode.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

fs/hfsplus/unicode.c +0 -0

index dfa90c2..2daf7b0 100644

fs/hfsplus/unicode.c int hfsplus_uni2asc(struct super_block *sb, · line 140

Quoted 7 lines

fs/hfsplus/unicode.c +1 -1

@@ -135,7 +135,7 @@ int hfsplus_uni2asc(struct super_block *sb,
  	ustrlen = be16_to_cpu(ustr->length);
  	len = *len_p;
  	ce1 = NULL;
-	compose = !test_bit(HFSPLUS_SB_NODECOMPOSE,
 &HFSPLUS_SB(sb)->flags);
+	compose = test_bit(HFSPLUS_SB_NODECOMPOSE,
 &HFSPLUS_SB(sb)->flags);

I'm not sure this is a mistake. The developers probably wanted the filenames to be recomposed before being presented in utf8. With your
patch,
if you try the following (with the default mount options):

  touch Á
  ls | hexdump -C

Quoted 7 lines

the utf8 output filename will be using the combining accent (CC
81)
instead
of the Á character (C3 81). This is a bit annoying because it won't
print
correctly in my terminal anymore.

I'm not exatly know why combining accent cannot print correctly in terminal
and how to avoid it. Whether apply my patch or not, my terminal cannot
print the hangul charactor correctly.

Viacheslav Dubeyko 於 2017-11-23 12:21 寫到:

Quoted 29 lines · tchou

Quoted 13 lines

Quoted 6 lines

Could you please share the examples of incorrect and correct behaviour?

Quoted 7 lines

What is it exactly that you are trying to fix? You mention an issue
with
hangul characters, but I failed to trigger it. Could you expand on
that?

Quoted 13 lines

Hi all,
There is an example.I use Mac mini to format my usb disk to HFS+ and
touch
the file with filename "공유"(a Korean actor, https://goo.gl/VcBsrn
)
on
it.
After it, I mount the usb disk on my ubuntu(Linux 4.14.0+) and get
the
following error message when trying to ls and cp the file:

Quoted 9 lines

Could you share the all mount options that you used? It looks that you
simply don't use the proper mount options set under the Linux. Because
MacOS X will mount properly always.

Thanks,
Vyacheslav Dubeyko.

Hi Vyacheslav,

I use the default mount option:

mount /dev/sdc1 /mnt/hfs
/dev/sdc1 on /mnt/hfs type hfsplus
(rw,relatime,umask=22,uid=0,gid=0,nls=utf8)

Thanks,
TCHou

Quoted 1 line

Quoted 39 lines

Viacheslav Dubeyko 於 2017-11-23 12:21 寫到:

Quoted 29 lines · tchou

Quoted 13 lines

Quoted 6 lines

Could you please share the examples of incorrect and correct behaviour?

Quoted 7 lines

What is it exactly that you are trying to fix? You mention an issue
with
hangul characters, but I failed to trigger it. Could you expand on
that?

Quoted 13 lines

Hi all,
There is an example.I use Mac mini to format my usb disk to HFS+ and
touch
the file with filename "공유"(a Korean actor, https://goo.gl/VcBsrn
)
on
it.
After it, I mount the usb disk on my ubuntu(Linux 4.14.0+) and get
the
following error message when trying to ls and cp the file:

Quoted 9 lines

Could you share the all mount options that you used? It looks that you
simply don't use the proper mount options set under the Linux. Because
MacOS X will mount properly always.

Thanks,
Vyacheslav Dubeyko.

Quoted 8 lines

Hi Vyacheslav,

I use the default mount option:

mount /dev/sdc1 /mnt/hfs
/dev/sdc1 on /mnt/hfs type hfsplus
(rw,relatime,umask=22,uid=0,gid=0,nls=utf8)

Please, take a look into mount options description [1]. Especially, take a look into decompose/nodecompose options. You simply needs to use the proper mount option for your case.

Thanks,
Vyacheslav Dubeyko.

[1] https://www.kernel.org/doc/Documentation/filesystems/hfsplus.txt

Quoted 3 lines

Thanks,
TCHou

Quoted 1 line

Hi:

your issue seems to be in the decomposition of hangul characters, not in
the recomposition before printing. The hfsplus module on linux is saving
the name of your actor as AC F5 C7 20, without performing any decomposition at all.

The reason your patch hides the bug is because it causes linux to present
filenames as decomposed utf8, so it is not necessary to decompose again
before working with them. But the issue is still there, and you will most
likely run into trouble if you make a hangul filename in linux and try
to work with it in MacOS.

Reviewing the code it would seem that the developers completely forgot
the hangul characters had their own rules for decomposition. It's weird
because they did the composition part correctly.

I've made a quick draft of a patch, mostly by copying the code provided
in the unicode web. I don't think we can actually use it on a

release,
but it should be enough to check if I'm right. It works fine on linux,
but I don't have a mac, so it would be great if you could test it for me.

Thanks,
Ernest

(By the way, there is no reason you should have to use the nodecompose
mount option, as the other reviewer suggested. Using that option will have a similar effect to that of your patch. It will hide the problem,
but if you create a hangul filename on linux with that option you probably won't be able to use it on a mac.)

---

fs/hfsplus/unicode.c +0 -0

index dfa90c2..9006c61 100644
@@ -272,7 +272,7 @@ static inline int asc2unichar(struct super_block
 *sb, const char *astr, int len,
  	return size;
  }

fs/hfsplus/unicode.c +1 -1

-/* Decomposes a single unicode character. */
+/* Decomposes a single non-Hangul unicode character. */

 static inline u16 *decompose_unichar(wchar_t uc, int *size) {
  int off;

fs/hfsplus/unicode.c +0 -0

@@ -296,6 +296,29 @@ static inline u16 *decompose_unichar(wchar_t uc,
 int *size)
  	return hfsplus_decompose_table + (off / 4);
  }

fs/hfsplus/unicode.c +23 -0

+/* Decomposes a Hangul unicode character. */
+int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+

 int hfsplus_asc2uni(struct super_block *sb,
      struct hfsplus_unistr *ustr, int max_unistr_len, const char *astr, int len)

fs/hfsplus/unicode.c +1 -0

@@ -303,15 +326,23 @@ int hfsplus_asc2uni(struct super_block *sb,
  	int size, dsize, decompose;
  	u16 *dstr, outlen = 0;
  	wchar_t c;
+	u16 hangul_buf[3];

  decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
  while (outlen < max_unistr_len && len > 0) { size = asc2unichar(sb, astr, len, &c);

fs/hfsplus/unicode.c +8 -3

-		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
-		else
+		if (decompose) {
+			/* Hangul is handled separately */
+			dstr = &hangul_buf[0];
+			dsize = decompose_hangul(c, dstr);
+			if (dsize == 0)
+				/* not Hangul */
+				dstr = decompose_unichar(c, &dsize);
+		} else {

  dstr = NULL;

fs/hfsplus/unicode.c +2 -0

+		}
+

  if (dstr) {
  if (outlen + dsize > max_unistr_len) break;

Quoted 25 lines · Ernesto A. Fernández

Hi:

your issue seems to be in the decomposition of hangul characters, not in
the recomposition before printing. The hfsplus module on linux is saving
the name of your actor as AC F5 C7 20, without performing any decomposition at all.

The reason your patch hides the bug is because it causes linux to present
filenames as decomposed utf8, so it is not necessary to decompose again
before working with them. But the issue is still there, and you will most
likely run into trouble if you make a hangul filename in linux and try
to work with it in MacOS.

Reviewing the code it would seem that the developers completely forgot
the hangul characters had their own rules for decomposition. It's weird
because they did the composition part correctly.

Quoted 3 lines

I've made a quick draft of a patch, mostly by copying the code provided
in the unicode web. I don't think we can actually use it on a

Could you please share the link for "the unicode web"?

Thanks,
Vyacheslav Dubeyko.

Quoted 93 lines

release,
but it should be enough to check if I'm right. It works fine on linux,
but I don't have a mac, so it would be great if you could test it for me.

Thanks,
Ernest

(By the way, there is no reason you should have to use the nodecompose
mount option, as the other reviewer suggested. Using that option will have a similar effect to that of your patch. It will hide the problem,
but if you create a hangul filename on linux with that option you probably won't be able to use it on a mac.)

---

fs/hfsplus/unicode.c +0 -0

index dfa90c2..9006c61 100644
@@ -272,7 +272,7 @@ static inline int asc2unichar(struct super_block
 *sb, const char *astr, int len,
  	return size;
  }

fs/hfsplus/unicode.c +1 -1

-/* Decomposes a single unicode character. */
+/* Decomposes a single non-Hangul unicode character. */

 static inline u16 *decompose_unichar(wchar_t uc, int *size) {
  int off;

fs/hfsplus/unicode.c +0 -0

@@ -296,6 +296,29 @@ static inline u16 *decompose_unichar(wchar_t uc,
 int *size)
  	return hfsplus_decompose_table + (off / 4);
  }

fs/hfsplus/unicode.c +23 -0

+/* Decomposes a Hangul unicode character. */
+int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+

 int hfsplus_asc2uni(struct super_block *sb,
      struct hfsplus_unistr *ustr, int max_unistr_len, const char *astr, int len)

fs/hfsplus/unicode.c +1 -0

@@ -303,15 +326,23 @@ int hfsplus_asc2uni(struct super_block *sb,
  	int size, dsize, decompose;
  	u16 *dstr, outlen = 0;
  	wchar_t c;
+	u16 hangul_buf[3];

  decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
  while (outlen < max_unistr_len && len > 0) { size = asc2unichar(sb, astr, len, &c);

fs/hfsplus/unicode.c +8 -3

-		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
-		else
+		if (decompose) {
+			/* Hangul is handled separately */
+			dstr = &hangul_buf[0];
+			dsize = decompose_hangul(c, dstr);
+			if (dsize == 0)
+				/* not Hangul */
+				dstr = decompose_unichar(c, &dsize);
+		} else {

  dstr = NULL;

fs/hfsplus/unicode.c +2 -0

+		}
+

  if (dstr) {
  if (outlen + dsize > max_unistr_len) break;

Hi:

your issue seems to be in the decomposition of hangul characters, not in the recomposition before printing. The hfsplus module on linux is saving the name of your actor as AC F5 C7 20, without performing any decomposition at all.

The reason your patch hides the bug is because it causes linux to present filenames as decomposed utf8, so it is not necessary to decompose again before working with them. But the issue is still there, and you will most likely run into trouble if you make a hangul filename in linux and try to work with it in MacOS.

Reviewing the code it would seem that the developers completely forgot the hangul characters had their own rules for decomposition. It's weird because they did the composition part correctly.

I've made a quick draft of a patch, mostly by copying the code provided in the unicode web. I don't think we can actually use it on a release, but it should be enough to check if I'm right. It works fine on linux, but I don't have a mac, so it would be great if you could test it for me.

Thanks,
Ernest

(By the way, there is no reason you should have to use the nodecompose mount option, as the other reviewer suggested. Using that option will have a similar effect to that of your patch. It will hide the problem, but if you create a hangul filename on linux with that option you probably won't be able to use it on a mac.)

---

fs/hfsplus/unicode.c +1 -1

index dfa90c2..9006c61 100644
@@ -272,7 +272,7 @@ static inline int asc2unichar(struct super_block
 *sb, const char *astr, int len,
 	return size;
 }
 
-/* Decomposes a single unicode character. */
+/* Decomposes a single non-Hangul unicode character. */
 static inline u16 *decompose_unichar(wchar_t uc, int *size)
 {

fs/hfsplus/unicode.c +23 -0

 	int off;

@@ -296,6 +296,29 @@ static inline u16 *decompose_unichar(wchar_t uc, 
 int *size)
 	return hfsplus_decompose_table + (off / 4);
 }
 
+/* Decomposes a Hangul unicode character. */
+int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+
 int hfsplus_asc2uni(struct super_block *sb,
 		    struct hfsplus_unistr *ustr, int max_unistr_len,

fs/hfsplus/unicode.c +11 -3

 		    const char *astr, int len)

@@ -303,15 +326,23 @@ int hfsplus_asc2uni(struct super_block *sb,
 	int size, dsize, decompose;
 	u16 *dstr, outlen = 0;
 	wchar_t c;
+	u16 hangul_buf[3];
 
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, 
 &HFSPLUS_SB(sb)->flags);
 	while (outlen < max_unistr_len && len > 0) {
 		size = asc2unichar(sb, astr, len, &c);
 
-		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
-		else
+		if (decompose) {
+			/* Hangul is handled separately */
+			dstr = &hangul_buf[0];
+			dsize = decompose_hangul(c, dstr);
+			if (dsize == 0)
+				/* not Hangul */
+				dstr = decompose_unichar(c, &dsize);
+		} else {
 			dstr = NULL;
+		}
+
 		if (dstr) {
 			if (outlen + dsize > max_unistr_len)

break;

Ernesto A. Fernández 於 2017-11-23 19:32 寫到:

Quoted 94 lines

Hi:

your issue seems to be in the decomposition of hangul characters, not in the recomposition before printing. The hfsplus module on linux is saving the name of your actor as AC F5 C7 20, without performing any decomposition at all.

The reason your patch hides the bug is because it causes linux to present filenames as decomposed utf8, so it is not necessary to decompose again before working with them. But the issue is still there, and you will most likely run into trouble if you make a hangul filename in linux and try to work with it in MacOS.

Reviewing the code it would seem that the developers completely forgot the hangul characters had their own rules for decomposition. It's weird because they did the composition part correctly.

I've made a quick draft of a patch, mostly by copying the code provided in the unicode web. I don't think we can actually use it on a release, but it should be enough to check if I'm right. It works fine on linux, but I don't have a mac, so it would be great if you could test it for me.

Thanks,
Ernest

(By the way, there is no reason you should have to use the nodecompose mount option, as the other reviewer suggested. Using that option will have a similar effect to that of your patch. It will hide the problem, but if you create a hangul filename on linux with that option you probably won't be able to use it on a mac.)

---

fs/hfsplus/unicode.c +1 -1

index dfa90c2..9006c61 100644
@@ -272,7 +272,7 @@ static inline int asc2unichar(struct super_block
 *sb, const char *astr, int len,
 	return size;
 }
 
-/* Decomposes a single unicode character. */
+/* Decomposes a single non-Hangul unicode character. */
 static inline u16 *decompose_unichar(wchar_t uc, int *size)
 {

	int off;

fs/hfsplus/unicode.c +23 -0

@@ -296,6 +296,29 @@ static inline u16 *decompose_unichar(wchar_t uc, int
 *size)
 	return hfsplus_decompose_table + (off / 4);
 }
 
+/* Decomposes a Hangul unicode character. */
+int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+
 int hfsplus_asc2uni(struct super_block *sb,
 		    struct hfsplus_unistr *ustr, int max_unistr_len,

const char *astr, int len)

fs/hfsplus/unicode.c +5 -3

@@ -303,15 +326,23 @@ int hfsplus_asc2uni(struct super_block *sb,
 	int size, dsize, decompose;
 	u16 *dstr, outlen = 0;
 	wchar_t c;
+	u16 hangul_buf[3];
 
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
 	while (outlen < max_unistr_len && len > 0) {
 		size = asc2unichar(sb, astr, len, &c);
 
-		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
-		else
+		if (decompose) {
+			/* Hangul is handled separately */
+			dstr = &hangul_buf[0];
+			dsize = decompose_hangul(c, dstr);

fs/hfsplus/unicode.c int hfsplus_asc2uni(struct super_block *sb, · line 348

Quoted 10 lines

fs/hfsplus/unicode.c +6 -0

+			if (dsize == 0)
+				/* not Hangul */
+				dstr = decompose_unichar(c, &dsize);
+		} else {
 			dstr = NULL;
+		}
+
 		if (dstr) {
 			if (outlen + dsize > max_unistr_len)
 				break;

Hi,

Thank you for your explanation and your draft.
I test four versions of hfsplus module:
--------------------------------
1. origin linux
2. apply my patch
3. mount with nodecompose option
4. apply your patch
--------------------------------

There are the results of the test:
---------------------------------------------------------------------
Linux can ls and cp the file from mac correctly with module 2,3,4. Mac cannot cp the files correctly which touched by module 1,2,3. Mac can cp the file correctly which touched by module with your patch. Mac can ls the files which touch by all modules.
---------------------------------------------------------------------

It seems that everything goes well with your patch. The result is just like you say, my patch or mount with nodecompose option cannot perform normally.

By the way, are the decompose procedures of function hfsplus_hash_dentry() and hfsplus_compare_dentry() need to modify as well?

Thanks,
TCHou

Quoted 108 lines

Ernesto A. Fernández 於 2017-11-23 19:32 寫到:

Quoted 94 lines

Hi:

your issue seems to be in the decomposition of hangul characters, not in the recomposition before printing. The hfsplus module on linux is saving the name of your actor as AC F5 C7 20, without performing any decomposition at all.

The reason your patch hides the bug is because it causes linux to present filenames as decomposed utf8, so it is not necessary to decompose again before working with them. But the issue is still there, and you will most likely run into trouble if you make a hangul filename in linux and try to work with it in MacOS.

Reviewing the code it would seem that the developers completely forgot the hangul characters had their own rules for decomposition. It's weird because they did the composition part correctly.

I've made a quick draft of a patch, mostly by copying the code provided in the unicode web. I don't think we can actually use it on a release, but it should be enough to check if I'm right. It works fine on linux, but I don't have a mac, so it would be great if you could test it for me.

Thanks,
Ernest

(By the way, there is no reason you should have to use the nodecompose mount option, as the other reviewer suggested. Using that option will have a similar effect to that of your patch. It will hide the problem, but if you create a hangul filename on linux with that option you probably won't be able to use it on a mac.)

---

fs/hfsplus/unicode.c +1 -1

index dfa90c2..9006c61 100644
@@ -272,7 +272,7 @@ static inline int asc2unichar(struct super_block
 *sb, const char *astr, int len,
 	return size;
 }
 
-/* Decomposes a single unicode character. */
+/* Decomposes a single non-Hangul unicode character. */
 static inline u16 *decompose_unichar(wchar_t uc, int *size)
 {

	int off;

fs/hfsplus/unicode.c +23 -0

@@ -296,6 +296,29 @@ static inline u16 *decompose_unichar(wchar_t uc, int
 *size)
 	return hfsplus_decompose_table + (off / 4);
 }
 
+/* Decomposes a Hangul unicode character. */
+int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+
 int hfsplus_asc2uni(struct super_block *sb,
 		    struct hfsplus_unistr *ustr, int max_unistr_len,

const char *astr, int len)

fs/hfsplus/unicode.c +5 -3

@@ -303,15 +326,23 @@ int hfsplus_asc2uni(struct super_block *sb,
 	int size, dsize, decompose;
 	u16 *dstr, outlen = 0;
 	wchar_t c;
+	u16 hangul_buf[3];
 
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
 	while (outlen < max_unistr_len && len > 0) {
 		size = asc2unichar(sb, astr, len, &c);
 
-		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
-		else
+		if (decompose) {
+			/* Hangul is handled separately */
+			dstr = &hangul_buf[0];
+			dsize = decompose_hangul(c, dstr);

fs/hfsplus/unicode.c int hfsplus_asc2uni(struct super_block *sb, · line 348

Quoted 10 lines

fs/hfsplus/unicode.c +6 -0

+			if (dsize == 0)
+				/* not Hangul */
+				dstr = decompose_unichar(c, &dsize);
+		} else {
 			dstr = NULL;
+		}
+
 		if (dstr) {
 			if (outlen + dsize > max_unistr_len)
 				break;

Hi,

Quoted 8 lines

Thank you for your explanation and your draft.
I test four versions of hfsplus module:
--------------------------------
1. origin linux
2. apply my patch
3. mount with nodecompose option
4. apply your patch
--------------------------------

It seems you were very thorough, thank you.

Quoted 8 lines

There are the results of the test:
---------------------------------------------------------------------
Linux can ls and cp the file from mac correctly with module 2,3,4. Mac cannot cp the files correctly which touched by module 1,2,3. Mac can cp the file correctly which touched by module with your patch. Mac can ls the files which touch by all modules.
---------------------------------------------------------------------

Quoted 3 lines

It seems that everything goes well with your patch. The result is just like you say, my patch or mount with nodecompose option cannot perform normally.

That's good to hear. I will submit a patch as soon as I can figure out the licensing issues. If you want I can credit you as reporter and tester.

Quoted 3 lines

By the way, are the decompose procedures of function hfsplus_hash_dentry() and hfsplus_compare_dentry() need to modify as well?

You are correct, good thing you noticed. I'll add that when I resend the patch.

Quoted 2 lines

Thanks,
TCHou

It would be good to have some feedback on the use of code under the Unicode License, I don't know if that can be a problem.

To Ting-Chang Hou: This patch is slightly different from the one you tested with your mac. You are still tagged as Tested-by, but you may want to check again that I have not broken anything.

Thanks,
Ernest

-- >8 --
Subject: [PATCH] hfsplus: fix decomposition of Hangul characters

Files created under macOS become essentially unusable under linux if their names contain any Hangul, and vice versa.

This happens because the normalization of Hangul characters is a special
case: it can be done algorithmically, without need of a table. The hfsplus module deals with Hangul correctly when composing, but completely forgets about it when performing a decomposition.

Solve this by using the Hangul decomposition function provided in the Unicode Standard. It's under the Unicode License, compatible with the GPL.

This patch will cause trouble for Hangul filenames already created by the module in the past. This shouldn't be a special concern because the main purpose of the module was always sharing with macOS. If a user actually needs to access such a file the nodecompose mount option should be enough.

Reported-by Ting-Chang Hou <[email protected]>

Tested-by Ting-Chang Hou <[email protected]>

Signed-off-by Ernesto A. Fernández <[email protected]>

---
 fs/hfsplus/unicode.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 6 deletions(-)

fs/hfsplus/unicode.c +31 -2

index dfa90c2..7ceae91 100644
@@ -272,8 +272,8 @@ static inline int asc2unichar(struct super_block *sb, const char *astr, int len,
 	return size;
 }
 
-/* Decomposes a single unicode character. */
-static inline u16 *decompose_unichar(wchar_t uc, int *size)
+/* Decomposes a non-Hangul unicode character. */
+static u16 *decompose_nonhangul(wchar_t uc, int *size)
 {
 	int off;
 

@@ -296,6 +296,51 @@ static inline u16 *decompose_unichar(wchar_t uc, int *size)
 	return hfsplus_decompose_table + (off / 4);
 }
 
+/*
+ * Decomposes a Hangul unicode character.
+ *
+ * This function was adapted from sample code in section 3.12 of the
+ * Unicode Standard, version 10.0.
+ *
+ * Copyright (C) 1991-2017 Unicode, Inc.  All rights reserved.  Distributed
+ * under the Terms of Use in http://www.unicode.org/copyright.html.
+ */
+static int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;

fs/hfsplus/unicode.c +12 -4

+	return result;
+}
+
 int hfsplus_asc2uni(struct super_block *sb,
 		    struct hfsplus_unistr *ustr, int max_unistr_len,
 		    const char *astr, int len)

@@ -303,13 +348,14 @@ int hfsplus_asc2uni(struct super_block *sb,
 	int size, dsize, decompose;
 	u16 *dstr, outlen = 0;
 	wchar_t c;
+	u16 dhangul[3];
 
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
 	while (outlen < max_unistr_len && len > 0) {
 		size = asc2unichar(sb, astr, len, &c);
 
 		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
+			dstr = decompose_unichar(c, &dsize, &dhangul[0]);
 		else
 			dstr = NULL;
 		if (dstr) {

@@ -344,6 +390,7 @@ int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str)
 	unsigned long hash;
 	wchar_t c;
 	u16 c2;
+	u16 dhangul[3];
 
 	casefold = test_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(sb)->flags);
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);

@@ -357,7 +404,7 @@ int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str)
 		len -= size;
 
 		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
+			dstr = decompose_unichar(c, &dsize, &dhangul[0]);
 		else
 			dstr = NULL;
 		if (dstr) {

@@ -396,6 +443,7 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 	const char *astr1, *astr2;
 	u16 c1, c2;
 	wchar_t c;
+	u16 dhangul_1[3], dhangul_2[3];
 
 	casefold = test_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(sb)->flags);
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);

@@ -413,7 +461,8 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 			len1 -= size;
 
 			if (decompose)
-				dstr1 = decompose_unichar(c, &dsize1);
+				dstr1 = decompose_unichar(c, &dsize1,
+							  &dhangul_1[0]);
 			if (!decompose || !dstr1) {
 				c1 = c;
 				dstr1 = &c1;

@@ -427,7 +476,8 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 			len2 -= size;
 
 			if (decompose)
-				dstr2 = decompose_unichar(c, &dsize2);
+				dstr2 = decompose_unichar(c, &dsize2,
+							  &dhangul_2[0]);
 			if (!decompose || !dstr2) {
 				c2 = c;
 				dstr2 = &c2;

Quoted 16 lines · Ernesto A. Fernández

It would be good to have some feedback on the use of code under the Unicode License, I don't know if that can be a problem.

To Ting-Chang Hou: This patch is slightly different from the one you tested with your mac. You are still tagged as Tested-by, but you may want to check again that I have not broken anything.

Thanks,
Ernest

-- >8 --
Subject: [PATCH] hfsplus: fix decomposition of Hangul characters

Files created under macOS become essentially unusable under linux if their names contain any Hangul, and vice versa.

Quoted 9 lines

This happens because the normalization of Hangul characters is a special
case: it can be done algorithmically, without need of a table. The hfsplus module deals with Hangul correctly when composing, but completely forgets about it when performing a decomposition.

Solve this by using the Hangul decomposition function provided in the Unicode Standard. It's under the Unicode License, compatible with the GPL.

I believe it makes sense to share more details about algorithm details. I think that the Unicode standard contains some explanation how to decompose correctly. It will be great to have some brief citation in the patch description.

Quoted 67 lines

This patch will cause trouble for Hangul filenames already created by the module in the past. This shouldn't be a special concern because the main purpose of the module was always sharing with macOS. If a user actually needs to access such a file the nodecompose mount option should be enough.

Reported-by Ting-Chang Hou <[email protected]>

Tested-by Ting-Chang Hou <[email protected]>

Signed-off-by Ernesto A. Fernández <[email protected]>

---
 fs/hfsplus/unicode.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 56 insertions(+), 6 deletions(-)

fs/hfsplus/unicode.c +37 -2

index dfa90c2..7ceae91 100644
@@ -272,8 +272,8 @@ static inline int asc2unichar(struct super_block *sb, const char *astr, int len,
 	return size;
 }
 
-/* Decomposes a single unicode character. */
-static inline u16 *decompose_unichar(wchar_t uc, int *size)
+/* Decomposes a non-Hangul unicode character. */
+static u16 *decompose_nonhangul(wchar_t uc, int *size)
 {
 	int off;
 

@@ -296,6 +296,51 @@ static inline u16 *decompose_unichar(wchar_t uc, int *size)
 	return hfsplus_decompose_table + (off / 4);
 }
 
+/*
+ * Decomposes a Hangul unicode character.
+ *
+ * This function was adapted from sample code in section 3.12 of the
+ * Unicode Standard, version 10.0.
+ *
+ * Copyright (C) 1991-2017 Unicode, Inc.  All rights reserved.  Distributed
+ * under the Terms of Use in http://www.unicode.org/copyright.html.
+ */
+static int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) / Hangul_TCount;
+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+
+/* Decomposes a single unicode character. */
+static u16 *decompose_unichar(wchar_t uc, int *size, u16 *hangul_buffer)
+{
+	u16 *result;

fs/hfsplus/unicode.c static inline u16 *decompose_unichar(wchar_t uc, int *size) · line 337

Quoted 4 lines

fs/hfsplus/unicode.c +4 -0

+
+	/* Hangul is handled separately */
+	result = hangul_buffer;
+	*size = decompose_hangul(uc, result);

Is it possible to distinguish the different cases beforehand and to select one proper function for execution without the execution both ones in the worst case?

fs/hfsplus/unicode.c static inline u16 *decompose_unichar(wchar_t uc, int *size) · line 340

Quoted 3 lines

fs/hfsplus/unicode.c +3 -0

+	if (*size == 0)
+		/* not Hangul */
+		result = decompose_nonhangul(uc, size);

I think it makes sense to use the brackets here. If I remember correctly, this is the code style's requirement.

Thanks,
Vyacheslav Dubeyko.

Quoted 67 lines

fs/hfsplus/unicode.c +12 -4

+	return result;
+}
+
 int hfsplus_asc2uni(struct super_block *sb,
 		    struct hfsplus_unistr *ustr, int max_unistr_len,
 		    const char *astr, int len)

@@ -303,13 +348,14 @@ int hfsplus_asc2uni(struct super_block *sb,
 	int size, dsize, decompose;
 	u16 *dstr, outlen = 0;
 	wchar_t c;
+	u16 dhangul[3];
 
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);
 	while (outlen < max_unistr_len && len > 0) {
 		size = asc2unichar(sb, astr, len, &c);
 
 		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
+			dstr = decompose_unichar(c, &dsize, &dhangul[0]);
 		else
 			dstr = NULL;
 		if (dstr) {

@@ -344,6 +390,7 @@ int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str)
 	unsigned long hash;
 	wchar_t c;
 	u16 c2;
+	u16 dhangul[3];
 
 	casefold = test_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(sb)->flags);
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);

@@ -357,7 +404,7 @@ int hfsplus_hash_dentry(const struct dentry *dentry, struct qstr *str)
 		len -= size;
 
 		if (decompose)
-			dstr = decompose_unichar(c, &dsize);
+			dstr = decompose_unichar(c, &dsize, &dhangul[0]);
 		else
 			dstr = NULL;
 		if (dstr) {

@@ -396,6 +443,7 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 	const char *astr1, *astr2;
 	u16 c1, c2;
 	wchar_t c;
+	u16 dhangul_1[3], dhangul_2[3];
 
 	casefold = test_bit(HFSPLUS_SB_CASEFOLD, &HFSPLUS_SB(sb)->flags);
 	decompose = !test_bit(HFSPLUS_SB_NODECOMPOSE, &HFSPLUS_SB(sb)->flags);

@@ -413,7 +461,8 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 			len1 -= size;
 
 			if (decompose)
-				dstr1 = decompose_unichar(c, &dsize1);
+				dstr1 = decompose_unichar(c, &dsize1,
+							  &dhangul_1[0]);
 			if (!decompose || !dstr1) {
 				c1 = c;
 				dstr1 = &c1;

@@ -427,7 +476,8 @@ int hfsplus_compare_dentry(const struct dentry *dentry,
 			len2 -= size;
 
 			if (decompose)
-				dstr2 = decompose_unichar(c, &dsize2);
+				dstr2 = decompose_unichar(c, &dsize2,
+							  &dhangul_2[0]);
 			if (!decompose || !dstr2) {
 				c2 = c;
 				dstr2 = &c2;

Quoted 36 lines · Viacheslav Dubeyko

Quoted 28 lines · Ernesto A. Fernández

It would be good to have some feedback on the use of code under the
Unicode License, I don't know if that can be a problem.

To Ting-Chang Hou: This patch is slightly different from the one you
tested with your mac. You are still tagged as Tested-by, but you may
want to check again that I have not broken anything.

Thanks,
Ernest

-- >8 --
Subject: [PATCH] hfsplus: fix decomposition of Hangul characters

Files created under macOS become essentially unusable under linux if
their names contain any Hangul, and vice versa.

This happens because the normalization of Hangul characters is a special
case: it can be done algorithmically, without need of a table. The
hfsplus module deals with Hangul correctly when composing, but completely forgets about it when performing a decomposition.

Quoted 6 lines

Solve this by using the Hangul decomposition function provided in the
Unicode Standard. It's under the Unicode License, compatible with the
GPL.

Quoted 6 lines

I believe it makes sense to share more details about algorithm details.
I think that the Unicode standard contains some explanation how to decompose correctly. It will be great to have some brief citation in the
patch description.

The algorithm is very simple, the best way to understand it is just looking at the code. I don't know the first thing about Korean writing, so
I don't think I should attempt to explain why the decomposition is done
this way. If somebody else is interested in the details, they can follow
the citation in the header comment of the decompose_hangul function.

Quoted 82 lines

Quoted 82 lines

This patch will cause trouble for Hangul filenames already created by
the module in the past. This shouldn't be a special concern because the
main purpose of the module was always sharing with macOS. If a user
actually needs to access such a file the nodecompose mount option should
be enough.

Reported-by Ting-Chang Hou <[email protected]>

Tested-by Ting-Chang Hou <[email protected]>

Signed-off-by Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.

com>
---
 fs/hfsplus/unicode.c | 62
+++++++++++++++++++++++++++++++++++++++++++++++-----

 1 file changed, 56 insertions(+), 6 deletions(-)

fs/hfsplus/unicode.c +0 -0

index dfa90c2..7ceae91 100644
@@ -272,8 +272,8 @@ static inline int asc2unichar(struct
 super_block *sb, const char *astr, int len,
  	return size;
  }

fs/hfsplus/unicode.c +2 -2

-/* Decomposes a single unicode character. */
-static inline u16 *decompose_unichar(wchar_t uc, int *size)
+/* Decomposes a non-Hangul unicode character. */
+static u16 *decompose_nonhangul(wchar_t uc, int *size)

 {
  int off;

fs/hfsplus/unicode.c +0 -0

@@ -296,6 +296,51 @@ static inline u16 *decompose_unichar(wchar_t
 uc, int *size)
  	return hfsplus_decompose_table + (off / 4);
  }

+/*
+ * Decomposes a Hangul unicode character.
+ *
+ * This function was adapted from sample code in section 3.12 of the
+ * Unicode Standard, version 10.0.
+ *
+ * Copyright (C) 1991-2017 Unicode, Inc.  All rights reserved.  Distributed
+ * under the Terms of Use in http://www.unicode.org/copyright.ht
ml.
+ */

fs/hfsplus/unicode.c +11 -0

+static int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) /

Hangul_TCount;

fs/hfsplus/unicode.c +13 -0

+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+
+/* Decomposes a single unicode character. */
+static u16 *decompose_unichar(wchar_t uc, int *size, u16

*hangul_buffer)

fs/hfsplus/unicode.c +2 -0

+{
+	u16 *result;

Quoted 8 lines

fs/hfsplus/unicode.c static inline u16 *decompose_unichar(wchar_t uc, int *size) · line 337

Quoted 4 lines

fs/hfsplus/unicode.c +4 -0

+
+	/* Hangul is handled separately */
+	result = hangul_buffer;
+	*size = decompose_hangul(uc, result);

Is it possible to distinguish the different cases beforehand and to select one proper function for execution without the execution both ones
in the worst case?

The decompose_hangul and decompose_nonhangul functions are called only once,
so I would expect the compiler to inline them both. The result should be
similar to your suggestion.

Quoted 7 lines

fs/hfsplus/unicode.c static inline u16 *decompose_unichar(wchar_t uc, int *size) · line 340

Quoted 5 lines

fs/hfsplus/unicode.c +3 -0

+	if (*size == 0)
+		/* not Hangul */
+		result = decompose_nonhangul(uc, size);

I think it makes sense to use the brackets here. If I remember correctly, this is the code style's requirement.

I couldn't find such a requirement, but I did find some other places within
the hfsplus code where the comments are done my way. For example, in the
hfsplus_cat_write_inode() function of the inode.c file.

Quoted 43 lines · Ernesto A. Fernández

Quoted 36 lines · Viacheslav Dubeyko

Quoted 28 lines · Ernesto A. Fernández

It would be good to have some feedback on the use of code under the
Unicode License, I don't know if that can be a problem.

To Ting-Chang Hou: This patch is slightly different from the one you
tested with your mac. You are still tagged as Tested-by, but you may
want to check again that I have not broken anything.

Thanks,
Ernest

-- >8 --
Subject: [PATCH] hfsplus: fix decomposition of Hangul characters

Files created under macOS become essentially unusable under linux if
their names contain any Hangul, and vice versa.

This happens because the normalization of Hangul characters is a special
case: it can be done algorithmically, without need of a table. The
hfsplus module deals with Hangul correctly when composing, but completely forgets about it when performing a decomposition.

Quoted 6 lines

Solve this by using the Hangul decomposition function provided in the
Unicode Standard. It's under the Unicode License, compatible with the
GPL.

Quoted 6 lines

I believe it makes sense to share more details about algorithm details.
I think that the Unicode standard contains some explanation how to decompose correctly. It will be great to have some brief citation in the
patch description.

Quoted 9 lines

The algorithm is very simple, the best way to understand it is just looking at the code. I don't know the first thing about Korean writing, so
I don't think I should attempt to explain why the decomposition is done
this way. If somebody else is interested in the details, they can follow
the citation in the header comment of the decompose_hangul function.

You should share the link where it can be found.

Finally, the comment
needs in more detailed explanation.

Quoted 90 lines

Quoted 82 lines

Quoted 82 lines

This patch will cause trouble for Hangul filenames already created by
the module in the past. This shouldn't be a special concern because the
main purpose of the module was always sharing with macOS. If a user
actually needs to access such a file the nodecompose mount option should
be enough.

Reported-by Ting-Chang Hou <[email protected]>

Tested-by Ting-Chang Hou <[email protected]>

Signed-off-by Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.

com>
---
 fs/hfsplus/unicode.c | 62
+++++++++++++++++++++++++++++++++++++++++++++++-----

 1 file changed, 56 insertions(+), 6 deletions(-)

fs/hfsplus/unicode.c +0 -0

index dfa90c2..7ceae91 100644
@@ -272,8 +272,8 @@ static inline int asc2unichar(struct
 super_block *sb, const char *astr, int len,
  	return size;
  }

fs/hfsplus/unicode.c +2 -2

-/* Decomposes a single unicode character. */
-static inline u16 *decompose_unichar(wchar_t uc, int *size)
+/* Decomposes a non-Hangul unicode character. */
+static u16 *decompose_nonhangul(wchar_t uc, int *size)

 {
  int off;

fs/hfsplus/unicode.c +0 -0

@@ -296,6 +296,51 @@ static inline u16 *decompose_unichar(wchar_t
 uc, int *size)
  	return hfsplus_decompose_table + (off / 4);
  }

+/*
+ * Decomposes a Hangul unicode character.
+ *
+ * This function was adapted from sample code in section 3.12 of the
+ * Unicode Standard, version 10.0.
+ *
+ * Copyright (C) 1991-2017 Unicode, Inc.  All rights reserved.  Distributed
+ * under the Terms of Use in http://www.unicode.org/copyright.ht
ml.
+ */

fs/hfsplus/unicode.c +11 -0

+static int decompose_hangul(wchar_t uc, u16 *result)
+{
+	int index;
+	int l, v, t;
+
+	index = uc - Hangul_SBase;
+	if (index < 0 || index >= Hangul_SCount)
+		return 0;
+
+	l = Hangul_LBase + index / Hangul_NCount;
+	v = Hangul_VBase + (index % Hangul_NCount) /

Hangul_TCount;

fs/hfsplus/unicode.c +13 -0

+	t = Hangul_TBase + index % Hangul_TCount;
+
+	result[0] = l;
+	result[1] = v;
+	if (t != Hangul_TBase) {
+		result[2] = t;
+		return 3;
+	}
+	return 2;
+}
+
+/* Decomposes a single unicode character. */
+static u16 *decompose_unichar(wchar_t uc, int *size, u16

*hangul_buffer)

fs/hfsplus/unicode.c +2 -0

+{
+	u16 *result;

Quoted 8 lines

fs/hfsplus/unicode.c static inline u16 *decompose_unichar(wchar_t uc, int *size) · line 337

Quoted 4 lines

fs/hfsplus/unicode.c +4 -0

+
+	/* Hangul is handled separately */
+	result = hangul_buffer;
+	*size = decompose_hangul(uc, result);

Is it possible to distinguish the different cases beforehand and to select one proper function for execution without the execution both ones
in the worst case?

Quoted 6 lines

The decompose_hangul and decompose_nonhangul functions are called only once,
so I would expect the compiler to inline them both. The result should be
similar to your suggestion.

It will be much better to have:

If (hangful) {
    <decompose_hangul>
} else {
    <decompose_nonhangul>
}

Currently, the code looks weird. Especially, decompose_hangul() is able to distinguish two different cases. So, it's easy to extract the check.

Quoted 7 lines

Quoted 7 lines

fs/hfsplus/unicode.c static inline u16 *decompose_unichar(wchar_t uc, int *size) · line 340

Quoted 5 lines

fs/hfsplus/unicode.c +3 -0

+	if (*size == 0)
+		/* not Hangul */
+		result = decompose_nonhangul(uc, size);

I think it makes sense to use the brackets here. If I remember correctly, this is the code style's requirement.

Quoted 6 lines

I couldn't find such a requirement, but I did find some other places within
the hfsplus code where the comments are done my way. For example, in the
hfsplus_cat_write_inode() function of the inode.c file.

The hfsplus code contains many awful places.

It doesn't mean that it
needs to follow the bad style examples. If you have only one string:

if (condition)
    <do_something>

then it's OK not to use the brackets. Otherwise, you should include the branch into the brackets.

Thanks,
Vyacheslav Dubeyko.

It would be good to have some feedback on the use of code under the Unicode License, I don't know if that can be a problem.

To Ting-Chang Hou: This patch is slightly different from the one you tested with your mac. You are still tagged as Tested-by, but you may want to check again that I have not broken anything.

Thanks,
Ernest

-- >8 --
Subject: [PATCH] hfsplus: fix decomposition of Hangul characters

Files created under macOS become essentially unusable under linux if their names contain any Hangul, and vice versa.

This happens because the normalization of Hangul characters is a special
case: it can be done algorithmically, without need of a table. The hfsplus module deals with Hangul correctly when composing, but completely forgets about it when performing a decomposition.

Solve this by using the Hangul decomposition function provided in the Unicode Standard. It's under the Unicode License, compatible with the GPL.

This patch will cause trouble for Hangul filenames already created by the module in the past. This shouldn't be a special concern because the main purpose of the module was always sharing with macOS. If a user actually needs to access such a file the nodecompose mount option should be enough.

Reported-by Ting-Chang Hou <[email protected]>

Tested-by Ting-Chang Hou <[email protected]>

Signed-off-by Ernesto A. Fernández <[email protected]>

Hi again:

Quoted 31 lines · Ernesto A. Fernández

It would be good to have some feedback on the use of code under the Unicode License, I don't know if that can be a problem.

To Ting-Chang Hou: This patch is slightly different from the one you tested with your mac. You are still tagged as Tested-by, but you may want to check again that I have not broken anything.

Thanks,
Ernest

-- >8 --
Subject: [PATCH] hfsplus: fix decomposition of Hangul characters

Files created under macOS become essentially unusable under linux if their names contain any Hangul, and vice versa.

This happens because the normalization of Hangul characters is a special
case: it can be done algorithmically, without need of a table. The hfsplus module deals with Hangul correctly when composing, but completely forgets about it when performing a decomposition.

Solve this by using the Hangul decomposition function provided in the Unicode Standard. It's under the Unicode License, compatible with the GPL.

This patch will cause trouble for Hangul filenames already created by the module in the past. This shouldn't be a special concern because the main purpose of the module was always sharing with macOS. If a user actually needs to access such a file the nodecompose mount option should be enough.

Quoted 4 lines

Reported-by Ting-Chang Hou <[email protected]>

Tested-by Ting-Chang Hou <[email protected]>

Signed-off-by Ernesto A. Fernández <[email protected]>

It took a while but this patch has been merged now. Thanks again for your report.

Ernest