[PATCH] Fix AppleDouble conversion in vfs_fruit

Ralph Böhme slow at samba.org
Wed Oct 10 06:01:25 UTC 2018


On Tue, Oct 09, 2018 at 02:24:33PM -0700, Jeremy Allison wrote:
>On Tue, Oct 09, 2018 at 01:03:50PM +0200, Ralph Böhme via samba-technical wrote:
>> On Tue, Oct 09, 2018 at 01:00:23PM +0200, Ralph Böhme via samba-technical wrote:
>> > On Tue, Oct 09, 2018 at 12:35:22PM +0200, Ralph Böhme wrote:
>> > > please hold on, something's fishy. This was passing manual make test
>> > > vfs.fruit whereas it's now failing. Checking...
>> >
>> > The culprit was a last minute change in 3/21 to use a pointer instead of
>> > an array to store the filler bytes. The problem is that whenever
>> > ad_pack() is called it overwrites the original filler bytes.
>> >
>> > So attached is the original version of the patchset without this change.
>> > Passes manual make test vfs.fruit again.
>> >
>> > CI: https://gitlab.com/samba-team/devel/samba/pipelines/32396036
>>
>> and here's the patch.
>
>Reviewing this - I have a question about the following.
>
>ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI -> ADEDOFF_RFORK_DOT_UND
>
>isn't what you're replacing here. You're replacing:
>
>ad_getentryoff(ad, ADEID_RFORK) -> ADEDOFF_RFORK_DOT_UND
>
>so either the code or the commit message isn't right.
>
>You are doing that replacement in patch
>
> [PATCH 13/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in ad_convert_move_reso()
>
>so this looks like a cut-and-paste commit message
>fail.
>
>Can you clarify ?

sorry, indeed a commit message copy&paste mistake.

But actually in 11/21

  vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in ad_convert_truncate()

13/21 looks correct.

Updated patchset attached.

Thanks!
-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46
-------------- next part --------------
From 65250acce46fa8caa1abb2db8cccf4c31d6616a4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 7 Oct 2018 18:26:47 +0200
Subject: [PATCH 01/21] s4:torture: FinderInfo conversion test with AppleDouble
 without xattr data

This testcase demonstrates that the AppleDouble conversion in vfs_fruit
doesn't correctly convert the FinderInfo data from the AppleDouble file
to a stream.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.vfs.fruit |   3 +
 source4/torture/vfs/fruit.c           | 258 ++++++++++++++++++++++++++++++++++
 2 files changed, 261 insertions(+)

diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index 8df25bccb79..bc46c2f4922 100644
--- a/selftest/knownfail.d/samba3.vfs.fruit
+++ b/selftest/knownfail.d/samba3.vfs.fruit
@@ -1 +1,4 @@
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\)
+^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
+^samba3.vfs.fruit metadata_stream.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
+^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index a9ae891a23f..58a94dd143c 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -886,6 +886,147 @@ static char osx_adouble_w_xattr[] = {
 	0x00, 0x00, 0x00, 0x1c, 0x00, 0x1e, 0xff, 0xff
 };
 
+/*
+ * The buf below contains the following AppleDouble encoded data:
+ *
+ * -------------------------------------------------------------------------------
+ * MagicNumber: 00051607                                        : AppleDouble
+ * Version    : 00020000                                        : Version 2
+ * Filler     : 4D 61 63 20 4F 53 20 58 20 20 20 20 20 20 20 20 : Mac OS X
+ * Num. of ent: 0002                                            : 2
+ *
+ * -------------------------------------------------------------------------------
+ * Entry ID   : 00000002 : Resource Fork
+ * Offset     : 00000052 : 82
+ * Length     : 0000011E : 286
+ *
+ * -RAW DUMP--:  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F : (ASCII)
+ * 00000000   : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................
+ * 00000010   : 54 68 69 73 20 72 65 73 6F 75 72 63 65 20 66 6F : This resource fo
+ * 00000020   : 72 6B 20 69 6E 74 65 6E 74 69 6F 6E 61 6C 6C 79 : rk intentionally
+ * 00000030   : 20 6C 65 66 74 20 62 6C 61 6E 6B 20 20 20 00 00 :  left blank   ..
+ * 00000040   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000050   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000060   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000070   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000080   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000090   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000A0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000B0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000C0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000D0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000E0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 000000F0   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ * 00000100   : 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 1E : ................
+ * 00000110   : 00 00 00 00 00 00 00 00 00 1C 00 1E FF FF       : ..............
+ *
+ * Entry ID   : 00000009 : Finder Info
+ * Offset     : 00000032 : 50
+ * Length     : 00000020 : 32
+ *
+ * -NOTE------: cannot detect whether FInfo or DInfo. assume FInfo.
+ *
+ * -FInfo-----:
+ * Type       : 57415645 : WAVE
+ * Creator    : 5054756C : PTul
+ * isAlias    : 0
+ * Invisible  : 0
+ * hasBundle  : 0
+ * nameLocked : 0
+ * Stationery : 0
+ * CustomIcon : 0
+ * Reserved   : 0
+ * Inited     : 0
+ * NoINITS    : 0
+ * Shared     : 0
+ * SwitchLaunc: 0
+ * Hidden Ext : 0
+ * color      : 000      : none
+ * isOnDesk   : 0
+ * Location v : 0000     : 0
+ * Location h : 0000     : 0
+ * Fldr       : 0000     : ..
+ *
+ * -FXInfo----:
+ * Rsvd|IconID: 0000     : 0
+ * Rsvd       : 0000     : ..
+ * Rsvd       : 0000     : ..
+ * Rsvd       : 0000     : ..
+ * AreInvalid : 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * CustomBadge: 0
+ * ObjctIsBusy: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * RoutingInfo: 0
+ * unknown bit: 0
+ * unknown bit: 0
+ * Rsvd|commnt: 0000     : 0
+ * PutAway    : 00000000 : 0
+ *
+ * -RAW DUMP--:  0  1  2  3  4  5  6  7  8  9  A  B  C  D  E  F : (ASCII)
+ * 00000000   : 57 41 56 45 50 54 75 6C 00 00 00 00 00 00 00 00 : WAVEPTul........
+ * 00000010   : 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 : ................
+ *  *
+ * It was created with:
+ * $ hexdump -ve '"\t" 7/1 "0x%02x, " 1/1 " 0x%02x," "\n"'
+ */
+static char osx_adouble_without_xattr[] = {
+	0x00, 0x05, 0x16, 0x07, 0x00, 0x02, 0x00, 0x00,
+	0x4d, 0x61, 0x63, 0x20, 0x4f, 0x53, 0x20, 0x58,
+	0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20, 0x20,
+	0x00, 0x02, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00,
+	0x00, 0x52, 0x00, 0x00, 0x01, 0x1e, 0x00, 0x00,
+	0x00, 0x09, 0x00, 0x00, 0x00, 0x32, 0x00, 0x00,
+	0x00, 0x20, 0x57, 0x41, 0x56, 0x45, 0x50, 0x54,
+	0x75, 0x6c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+	0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x1e, 0x54, 0x68, 0x69, 0x73, 0x20, 0x72,
+	0x65, 0x73, 0x6f, 0x75, 0x72, 0x63, 0x65, 0x20,
+	0x66, 0x6f, 0x72, 0x6b, 0x20, 0x69, 0x6e, 0x74,
+	0x65, 0x6e, 0x74, 0x69, 0x6f, 0x6e, 0x61, 0x6c,
+	0x6c, 0x79, 0x20, 0x6c, 0x65, 0x66, 0x74, 0x20,
+	0x62, 0x6c, 0x61, 0x6e, 0x6b, 0x20, 0x20, 0x20,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+	0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x1c, 0x00, 0x1e, 0xff, 0xff
+};
+
 /**
  * talloc and intialize an AfpInfo
  **/
@@ -2021,6 +2162,122 @@ static bool test_adouble_conversion(struct torture_context *tctx,
 	return ret;
 }
 
+/*
+ * Test conversion of AppleDouble file without embedded xattr data
+ */
+static bool test_adouble_conversion_wo_xattr(struct torture_context *tctx,
+					     struct smb2_tree *tree)
+{
+	TALLOC_CTX *mem_ctx = talloc_new(tctx);
+	const char *fname = BASEDIR "\\test_adouble_conversion";
+	const char *adname = BASEDIR "/._test_adouble_conversion";
+	NTSTATUS status;
+	struct smb2_handle testdirh;
+	bool ret = true;
+	const char *streams[] = {
+		"::$DATA",
+		AFPINFO_STREAM,
+		AFPRESOURCE_STREAM
+	};
+	struct smb2_create create;
+	struct smb2_find find;
+	unsigned int count;
+	union smb_search_data *d;
+	const char *data = "This resource fork intentionally left blank";
+	size_t datalen = strlen(data);
+
+	smb2_deltree(tree, BASEDIR);
+
+	status = torture_smb2_testdir(tree, BASEDIR, &testdirh);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"torture_smb2_testdir failed\n");
+	smb2_util_close(tree, testdirh);
+
+	ret = torture_setup_file(tctx, tree, fname, false);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "torture_setup_file failed\n");
+
+	ret = torture_setup_file(tctx, tree, adname, false);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "torture_setup_file failed\n");
+
+	ret = write_stream(tree, __location__, tctx, mem_ctx,
+			   adname, NULL, 0,
+			   sizeof(osx_adouble_without_xattr),
+			   osx_adouble_without_xattr);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "write_stream failed\n");
+
+	ret = enable_aapl(tctx, tree);
+	torture_assert_goto(tctx, ret == true, ret, done, "enable_aapl failed");
+
+	/*
+	 * Issue a smb2_find(), this triggers the server-side conversion
+	 */
+
+	create = (struct smb2_create) {
+		.in.desired_access = SEC_RIGHTS_DIR_READ,
+		.in.create_options = NTCREATEX_OPTIONS_DIRECTORY,
+		.in.file_attributes = FILE_ATTRIBUTE_DIRECTORY,
+		.in.share_access = NTCREATEX_SHARE_ACCESS_READ,
+		.in.create_disposition = NTCREATEX_DISP_OPEN,
+		.in.impersonation_level = SMB2_IMPERSONATION_ANONYMOUS,
+		.in.fname = BASEDIR,
+	};
+
+	status = smb2_create(tree, tctx, &create);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_create failed\n");
+
+	find = (struct smb2_find) {
+		.in.file.handle = create.out.file.handle,
+		.in.pattern = "*",
+		.in.max_response_size = 0x1000,
+		.in.level = SMB2_FIND_ID_BOTH_DIRECTORY_INFO,
+	};
+
+	status = smb2_find_level(tree, tree, &find, &count, &d);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_find_level failed\n");
+
+	status = smb2_util_close(tree, create.out.file.handle);
+	torture_assert_ntstatus_ok_goto(tctx, status, ret, done,
+					"smb2_util_close failed");
+
+	/*
+	 * Check number of streams
+	 */
+
+	ret = check_stream_list(tree, tctx, fname, 3, streams, false);
+	torture_assert_goto(tctx, ret == true, ret, done, "check_stream_list");
+
+
+	/*
+	 * Check Resourcefork data can be read.
+	 */
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPRESOURCE_STREAM,
+			   16, datalen, 0, datalen, data);
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "check AFPRESOURCE_STREAM failed\n");
+
+	/*
+	 * Check FinderInfo data has been migrated to stream.
+	 */
+
+	ret = check_stream(tree, __location__, tctx, mem_ctx,
+			   fname, AFPINFO_STREAM,
+			   0, 60, 16, 8, "WAVEPTul");
+	torture_assert_goto(tctx, ret == true, ret, done,
+			    "check AFPINFO_STREAM failed\n");
+
+done:
+	smb2_deltree(tree, BASEDIR);
+	talloc_free(mem_ctx);
+	return ret;
+}
+
 static bool test_aapl(struct torture_context *tctx,
 		      struct smb2_tree *tree)
 {
@@ -4866,6 +5123,7 @@ struct torture_suite *torture_vfs_fruit(TALLOC_CTX *ctx)
 	torture_suite_add_1smb2_test(suite, "copy-chunk streams", test_copy_chunk_streams);
 	torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion", test_adouble_conversion);
 	torture_suite_add_1smb2_test(suite, "NFS ACE entries", test_nfs_aces);
+	torture_suite_add_1smb2_test(suite, "OS X AppleDouble file conversion without embedded xattr", test_adouble_conversion_wo_xattr);
 
 	return suite;
 }
-- 
2.13.6


From 5c3f7c615e91f86a9732b89b914e426adf1b47e4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 11 Sep 2018 14:05:43 +0200
Subject: [PATCH 02/21] vfs_fruit: fix two comments

Thanks to the recent addition of ad_convert_xattr() we now correctly
handle this case.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 894c361a05f..65d6618ff2d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1055,8 +1055,7 @@ static bool ad_convert_xattr(struct adouble *ad,
  * Convert from Apple's ._ file to Netatalk
  *
  * Apple's AppleDouble may contain a FinderInfo entry longer then 32
- * bytes containing packed xattrs. Netatalk can't deal with that, so
- * we simply discard the packed xattrs.
+ * bytes containing packed xattrs.
  *
  * @return -1 in case an error occurred, 0 if no conversion was done, 1
  * otherwise
@@ -1374,9 +1373,7 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 
 	/*
 	 * Try to fixup AppleDouble files created by OS X with xattrs
-	 * appended to the ADEID_FINDERI entry. We simply remove the
-	 * xattrs blob, this means any fancy xattr that was stored
-	 * there is lost.
+	 * appended to the ADEID_FINDERI entry.
 	 */
 
 	ret = ad_convert(ad, smb_fname, ad->ad_fd);
-- 
2.13.6


From c4813e5e0ead77c55d5cadd1d6dee7ce54a3662e Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 15:12:44 +0200
Subject: [PATCH 03/21] vfs_fruit: store filler bytes from AppleDouble file
 header in struct adouble

This can later be used to distinguish between macOS created AppleDouble
files and AppleDouble files created by Samba or Netatalk.

macOS:    "Mac OS X        "
Samba:    "Netatalk        "
Netatalk: "Netatalk        "

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 65d6618ff2d..057e4e4265b 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -262,6 +262,7 @@ typedef enum {ADOUBLE_META, ADOUBLE_RSRC} adouble_type_t;
 #define ADEDLEN_VERSION     4
 #define ADEDLEN_FILLER      16
 #define AD_FILLER_TAG       "Netatalk        " /* should be 16 bytes */
+#define AD_FILLER_TAG_OSX   "Mac OS X        " /* should be 16 bytes */
 #define ADEDLEN_NENTRIES    2
 #define AD_HEADER_LEN       (ADEDLEN_MAGIC + ADEDLEN_VERSION + \
 			     ADEDLEN_FILLER + ADEDLEN_NENTRIES) /* 26 */
@@ -414,6 +415,7 @@ struct adouble {
 	adouble_type_t            ad_type;
 	uint32_t                  ad_magic;
 	uint32_t                  ad_version;
+	uint8_t                   ad_filler[ADEDLEN_FILLER];
 	struct ad_entry           ad_eid[ADEID_MAX];
 	char                     *ad_data;
 	struct ad_xattr_header    adx_header;
@@ -837,6 +839,8 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 		return false;
 	}
 
+	memcpy(ad->ad_filler, ad->ad_data + ADEDOFF_FILLER, ADEDLEN_FILLER);
+
 	adentries = RSVAL(ad->ad_data, ADEDOFF_NENTRIES);
 	if (adentries != nentries) {
 		DEBUG(1, ("invalid number of entries: %zu\n",
-- 
2.13.6


From 40dd4d79cea164f90a9d874b9787ba46f2c5476c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 2 Oct 2018 14:51:05 +0200
Subject: [PATCH 04/21] vfs_fruit: move setting ADEID_FINDERI length to
 ad_convert_xattr()

ad_convert_xattr() does the conversion of the xattr data in the
AppleDouble file, so we should update it's size there and should not
defer it to the caller.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 057e4e4265b..2dec9089006 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1052,6 +1052,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		fsp = NULL;
 	}
 
+	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	return true;
 }
 
@@ -1096,7 +1097,6 @@ static int ad_convert(struct adouble *ad,
 			ad_getentrylen(ad, ADEID_RFORK));
 	}
 
-	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
 	ad_setentryoff(ad, ADEID_RFORK,
 		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
 
-- 
2.13.6


From 163ffc54eddde63fe388fbeff7343eb4584349bc Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Oct 2018 08:23:59 +0200
Subject: [PATCH 05/21] vfs_fruit: do direct return from error checks in
 ad_convert()

Subsequent commits will move the mmap() into the subfunctions. This
change just prepares for that.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 2dec9089006..7d544ea7f94 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1081,8 +1081,7 @@ static int ad_convert(struct adouble *ad,
 	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
 	if (map == MAP_FAILED) {
 		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
-		rc = -1;
-		goto exit;
+		return -1;
 	}
 
 	ok = ad_convert_xattr(ad, smb_fname, map);
@@ -1106,12 +1105,18 @@ static int ad_convert(struct adouble *ad,
 	 */
 	rc = ftruncate(fd, ad_getentryoff(ad, ADEID_RFORK)
 		       + ad_getentrylen(ad, ADEID_RFORK));
-
-exit:
-	if (map != MAP_FAILED) {
+	if (rc != 0) {
 		munmap(map, origlen);
+		return -1;
 	}
-	return rc;
+
+	rc = munmap(map, origlen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return -1;
+	}
+
+	return 0;
 }
 
 /**
-- 
2.13.6


From 13f2062cedd4c14942b773744667e0961904cf74 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 4 Oct 2018 08:51:28 +0200
Subject: [PATCH 06/21] vfs_fruit: remove unneeded fd argument from
 ad_convert()

Use the struct adouble member ad_fd instead of passing it as an
argument. Who did that in the first place? :)

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 7d544ea7f94..7ed5830223d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1066,8 +1066,7 @@ static bool ad_convert_xattr(struct adouble *ad,
  * otherwise
  **/
 static int ad_convert(struct adouble *ad,
-		      const struct smb_filename *smb_fname,
-		      int fd)
+		      const struct smb_filename *smb_fname)
 {
 	int rc = 0;
 	char *map = MAP_FAILED;
@@ -1078,7 +1077,8 @@ static int ad_convert(struct adouble *ad,
 		ad_getentrylen(ad, ADEID_RFORK);
 
 	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
 	if (map == MAP_FAILED) {
 		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
 		return -1;
@@ -1103,7 +1103,7 @@ static int ad_convert(struct adouble *ad,
 	 * FIXME: direct ftruncate(), but we don't have a fsp for the
 	 * VFS call
 	 */
-	rc = ftruncate(fd, ad_getentryoff(ad, ADEID_RFORK)
+	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
 		       + ad_getentrylen(ad, ADEID_RFORK));
 	if (rc != 0) {
 		munmap(map, origlen);
@@ -1385,7 +1385,7 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 	 * appended to the ADEID_FINDERI entry.
 	 */
 
-	ret = ad_convert(ad, smb_fname, ad->ad_fd);
+	ret = ad_convert(ad, smb_fname);
 	if (ret != 0) {
 		DBG_WARNING("Failed to convert [%s]\n", smb_fname->base_name);
 		return len;
-- 
2.13.6


From 23e6d3972af7146ecc95875c6811ffeab9274f43 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:14:40 +0200
Subject: [PATCH 07/21] vfs_fruit: move storing of modified struct adouble to
 ad_convert()

ad_convert() modified it, so let ad_convert() also save it to disk. No
change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 7ed5830223d..57d3c800260 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1071,6 +1071,7 @@ static int ad_convert(struct adouble *ad,
 	int rc = 0;
 	char *map = MAP_FAILED;
 	size_t origlen;
+	ssize_t len;
 	bool ok;
 
 	origlen = ad_getentryoff(ad, ADEID_RFORK) +
@@ -1116,6 +1117,18 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return -1;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1391,18 +1404,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return len;
 	}
 
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return -1;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return -1;
-	}
-
 	p_ad = ad_get_entry(ad, ADEID_FINDERI);
 	if (p_ad == NULL) {
 		return -1;
-- 
2.13.6


From b8addd5bfbeabee4f586dbdcbdf841881e3bf131 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:25:27 +0200
Subject: [PATCH 08/21] vfs_fruit: move FinderInfo conversion to helper
 function and call it from ad_convert()

No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 192 ++++++++++++++++++++++++--------------------
 1 file changed, 104 insertions(+), 88 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 57d3c800260..5706ed1eae2 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1056,6 +1056,102 @@ static bool ad_convert_xattr(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_finderinfo(struct adouble *ad,
+				  const struct smb_filename *smb_fname)
+{
+	char *p_ad = NULL;
+	AfpInfo *ai = NULL;
+	DATA_BLOB aiblob;
+	struct smb_filename *stream_name = NULL;
+	files_struct *fsp = NULL;
+	size_t size;
+	ssize_t nwritten;
+	NTSTATUS status;
+	int saved_errno = 0;
+
+	p_ad = ad_get_entry(ad, ADEID_FINDERI);
+	if (p_ad == NULL) {
+		return false;
+	}
+
+	ai = afpinfo_new(talloc_tos());
+	if (ai == NULL) {
+		return false;
+	}
+
+	memcpy(ai->afpi_FinderInfo, p_ad, ADEDLEN_FINDERI);
+
+	aiblob = data_blob_talloc(talloc_tos(), NULL, AFP_INFO_SIZE);
+	if (aiblob.data == NULL) {
+		TALLOC_FREE(ai);
+		return false;
+	}
+
+	size = afpinfo_pack(ai, (char *)aiblob.data);
+	TALLOC_FREE(ai);
+	if (size != AFP_INFO_SIZE) {
+		return false;
+	}
+
+	stream_name = synthetic_smb_fname(talloc_tos(),
+					  smb_fname->base_name,
+					  AFPINFO_STREAM,
+					  NULL,
+					  smb_fname->flags);
+	if (stream_name == NULL) {
+		data_blob_free(&aiblob);
+		DBG_ERR("synthetic_smb_fname failed\n");
+		return false;
+	}
+
+	DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
+
+	status = SMB_VFS_CREATE_FILE(
+		ad->ad_handle->conn,		/* conn */
+		NULL,				/* req */
+		0,				/* root_dir_fid */
+		stream_name,			/* fname */
+		FILE_GENERIC_WRITE,		/* access_mask */
+		FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
+		FILE_OPEN_IF,			/* create_disposition */
+		0,				/* create_options */
+		0,				/* file_attributes */
+		INTERNAL_OPEN_ONLY,		/* oplock_request */
+		NULL,				/* lease */
+		0,				/* allocation_size */
+		0,				/* private_flags */
+		NULL,				/* sd */
+		NULL,				/* ea_list */
+		&fsp,				/* result */
+		NULL,				/* psbuf */
+		NULL, NULL);			/* create context */
+	TALLOC_FREE(stream_name);
+	if (!NT_STATUS_IS_OK(status)) {
+		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
+		return false;
+	}
+
+	nwritten = SMB_VFS_PWRITE(fsp,
+				  aiblob.data,
+				  aiblob.length,
+				  0);
+	if (nwritten == -1) {
+		DBG_ERR("SMB_VFS_PWRITE failed\n");
+		saved_errno = errno;
+		close_file(NULL, fsp, ERROR_CLOSE);
+		errno = saved_errno;
+		return false;
+	}
+
+	status = close_file(NULL, fsp, NORMAL_CLOSE);
+	if (!NT_STATUS_IS_OK(status)) {
+		return false;
+	}
+	fsp = NULL;
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1129,6 +1225,13 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
+	ok = ad_convert_finderinfo(ad, smb_fname);
+	if (!ok) {
+		DBG_ERR("Failed to convert [%s]\n",
+			smb_fname_str_dbg(smb_fname));
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -1325,15 +1428,8 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 {
 	SMB_STRUCT_STAT sbuf;
 	char *p_ad = NULL;
-	AfpInfo *ai = NULL;
-	DATA_BLOB aiblob;
-	struct smb_filename *stream_name = NULL;
-	files_struct *fsp = NULL;
-	ssize_t len;
 	size_t size;
-	ssize_t nwritten;
-	NTSTATUS status;
-	int saved_errno = 0;
+	ssize_t len;
 	int ret;
 	bool ok;
 
@@ -1404,86 +1500,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return len;
 	}
 
-	p_ad = ad_get_entry(ad, ADEID_FINDERI);
-	if (p_ad == NULL) {
-		return -1;
-	}
-
-	ai = afpinfo_new(talloc_tos());
-	if (ai == NULL) {
-		return -1;
-	}
-
-	memcpy(ai->afpi_FinderInfo, p_ad, ADEDLEN_FINDERI);
-
-	aiblob = data_blob_talloc(talloc_tos(), NULL, AFP_INFO_SIZE);
-	if (aiblob.data == NULL) {
-		TALLOC_FREE(ai);
-		return -1;
-	}
-
-	size = afpinfo_pack(ai, (char *)aiblob.data);
-	TALLOC_FREE(ai);
-	if (size != AFP_INFO_SIZE) {
-		return -1;
-	}
-
-	stream_name = synthetic_smb_fname(talloc_tos(),
-					  smb_fname->base_name,
-					  AFPINFO_STREAM,
-					  NULL,
-					  smb_fname->flags);
-	if (stream_name == NULL) {
-		data_blob_free(&aiblob);
-		DBG_ERR("synthetic_smb_fname failed\n");
-		return -1;
-	}
-
-	DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
-
-	status = SMB_VFS_CREATE_FILE(
-		ad->ad_handle->conn,		/* conn */
-		NULL,				/* req */
-		0,				/* root_dir_fid */
-		stream_name,			/* fname */
-		FILE_GENERIC_WRITE,		/* access_mask */
-		FILE_SHARE_READ | FILE_SHARE_WRITE, /* share_access */
-		FILE_OPEN_IF,			/* create_disposition */
-		0,				/* create_options */
-		0,				/* file_attributes */
-		INTERNAL_OPEN_ONLY,		/* oplock_request */
-		NULL,				/* lease */
-		0,				/* allocation_size */
-		0,				/* private_flags */
-		NULL,				/* sd */
-		NULL,				/* ea_list */
-		&fsp,				/* result */
-		NULL,				/* psbuf */
-		NULL, NULL);			/* create context */
-	TALLOC_FREE(stream_name);
-	if (!NT_STATUS_IS_OK(status)) {
-		DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-		return -1;
-	}
-
-	nwritten = SMB_VFS_PWRITE(fsp,
-				  aiblob.data,
-				  aiblob.length,
-				  0);
-	if (nwritten == -1) {
-		DBG_ERR("SMB_VFS_PWRITE failed\n");
-		saved_errno = errno;
-		close_file(NULL, fsp, ERROR_CLOSE);
-		errno = saved_errno;
-		return -1;
-	}
-
-	status = close_file(NULL, fsp, NORMAL_CLOSE);
-	if (!NT_STATUS_IS_OK(status)) {
-		return -1;
-	}
-	fsp = NULL;
-
 	return len;
 }
 
-- 
2.13.6


From bed21cdab7b7a088820d0f146f11d5208968bc37 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:26:46 +0200
Subject: [PATCH 09/21] vfs_fruit: move FinderInfo lenght check to ad_convert()

The final step in consolidating all conversion related work in
ad_convert(). No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 5706ed1eae2..73605c63445 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1170,6 +1170,10 @@ static int ad_convert(struct adouble *ad,
 	ssize_t len;
 	bool ok;
 
+	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
+		return 0;
+	}
+
 	origlen = ad_getentryoff(ad, ADEID_RFORK) +
 		ad_getentrylen(ad, ADEID_RFORK);
 
@@ -1485,10 +1489,6 @@ static ssize_t ad_read_rsrc_adouble(struct adouble *ad,
 		return -1;
 	}
 
-	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
-		return len;
-	}
-
 	/*
 	 * Try to fixup AppleDouble files created by OS X with xattrs
 	 * appended to the ADEID_FINDERI entry.
-- 
2.13.6


From 778ffde8e1167397de3639ee8cb150a43788a56a Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:13:16 +0200
Subject: [PATCH 10/21] vfs_fruit: split out truncating from ad_convert()

This may look a little ill-advised as this increases line count, but
the goal here is modularizing ad_convert() itself and making it as slick
as possible helps achieving that goal.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 73605c63445..41476625b33 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1152,6 +1152,24 @@ static bool ad_convert_finderinfo(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_truncate(struct adouble *ad,
+				const struct smb_filename *smb_fname)
+{
+	int rc;
+
+	/*
+	 * FIXME: direct ftruncate(), but we don't have a fsp for the
+	 * VFS call
+	 */
+	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
+		       + ad_getentrylen(ad, ADEID_RFORK));
+	if (rc != 0) {
+		return false;
+	}
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1200,13 +1218,8 @@ static int ad_convert(struct adouble *ad,
 	ad_setentryoff(ad, ADEID_RFORK,
 		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
 
-	/*
-	 * FIXME: direct ftruncate(), but we don't have a fsp for the
-	 * VFS call
-	 */
-	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
-		       + ad_getentrylen(ad, ADEID_RFORK));
-	if (rc != 0) {
+	ok = ad_convert_truncate(ad, smb_fname);
+	if (!ok) {
 		munmap(map, origlen);
 		return -1;
 	}
-- 
2.13.6


From 262d6b3d9ce697ea3fbdd8cc54c2a6ef5b1da773 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:15:04 +0200
Subject: [PATCH 11/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in
 ad_convert_truncate()

We really want the fixed size offset here, not a calculated one. Note
that "ad_getentryoff(ad, ADEID_RFORK)" is equal to ADEDOFF_RFORK_DOT_UND
in this case.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 41476625b33..02db5923c6a 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1161,8 +1161,8 @@ static bool ad_convert_truncate(struct adouble *ad,
 	 * FIXME: direct ftruncate(), but we don't have a fsp for the
 	 * VFS call
 	 */
-	rc = ftruncate(ad->ad_fd, ad_getentryoff(ad, ADEID_RFORK)
-		       + ad_getentrylen(ad, ADEID_RFORK));
+	rc = ftruncate(ad->ad_fd, ADEDOFF_RFORK_DOT_UND +
+		       ad_getentrylen(ad, ADEID_RFORK));
 	if (rc != 0) {
 		return false;
 	}
-- 
2.13.6


From b1096d24d2811a4686310dde974a8e799e5342be Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:44:53 +0200
Subject: [PATCH 12/21] vfs_fruit: split out moving of the resource fork

No change in behaviour.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 02db5923c6a..0aeed3a66bb 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1170,6 +1170,24 @@ static bool ad_convert_truncate(struct adouble *ad,
 	return true;
 }
 
+static bool ad_convert_move_reso(struct adouble *ad,
+				 const struct smb_filename *smb_fname,
+				 char *map)
+{
+	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
+		return true;
+	}
+
+	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+		map + ad_getentryoff(ad, ADEID_RFORK),
+		ad_getentrylen(ad, ADEID_RFORK));
+
+	ad_setentryoff(ad, ADEID_RFORK,
+		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
+
+	return true;
+}
+
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1209,15 +1227,12 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	if (ad_getentrylen(ad, ADEID_RFORK) > 0) {
-		memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
-			map + ad_getentryoff(ad, ADEID_RFORK),
-			ad_getentrylen(ad, ADEID_RFORK));
+	ok = ad_convert_move_reso(ad, smb_fname, map);
+	if (!ok) {
+		munmap(map, origlen);
+		return -1;
 	}
 
-	ad_setentryoff(ad, ADEID_RFORK,
-		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
-
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
 		munmap(map, origlen);
-- 
2.13.6


From aa160ecd240363f133cb8eb76589463c9e233a32 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 19:15:04 +0200
Subject: [PATCH 13/21] vfs_fruit: use ADEDOFF_RFORK_DOT_UND offset macro in
 ad_convert_move_reso()

We really want the fixed size offset here, not a calculated one. Note
that "ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI" is equal to
ADEDOFF_RFORK_DOT_UND.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 0aeed3a66bb..4c710bd2f99 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1178,12 +1178,11 @@ static bool ad_convert_move_reso(struct adouble *ad,
 		return true;
 	}
 
-	memmove(map + ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI,
+	memmove(map + ADEDOFF_RFORK_DOT_UND,
 		map + ad_getentryoff(ad, ADEID_RFORK),
 		ad_getentrylen(ad, ADEID_RFORK));
 
-	ad_setentryoff(ad, ADEID_RFORK,
-		       ad_getentryoff(ad, ADEID_FINDERI) + ADEDLEN_FINDERI);
+	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
 	return true;
 }
-- 
2.13.6


From acc20ebfd4a4581153dc940b7057d84761135c65 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:52:32 +0200
Subject: [PATCH 14/21] vfs_fruit: fix error returns in ad_convert_xattr()

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 4c710bd2f99..aaaf5fde97e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -985,14 +985,14 @@ static bool ad_convert_xattr(struct adouble *ad,
 		    !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
 		{
 			DBG_ERR("string_replace_allocate failed\n");
-			return -1;
+			return false;
 		}
 
 		tmp = mapped_name;
 		mapped_name = talloc_asprintf(talloc_tos(), ":%s", tmp);
 		TALLOC_FREE(tmp);
 		if (mapped_name == NULL) {
-			return -1;
+			return false;
 		}
 
 		stream_name = synthetic_smb_fname(talloc_tos(),
@@ -1003,7 +1003,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mapped_name);
 		if (stream_name == NULL) {
 			DBG_ERR("synthetic_smb_fname failed\n");
-			return -1;
+			return false;
 		}
 
 		DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
@@ -1030,7 +1030,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(stream_name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-			return -1;
+			return false;
 		}
 
 		nwritten = SMB_VFS_PWRITE(fsp,
@@ -1042,12 +1042,12 @@ static bool ad_convert_xattr(struct adouble *ad,
 			saved_errno = errno;
 			close_file(NULL, fsp, ERROR_CLOSE);
 			errno = saved_errno;
-			return -1;
+			return false;
 		}
 
 		status = close_file(NULL, fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			return -1;
+			return false;
 		}
 		fsp = NULL;
 	}
-- 
2.13.6


From 074293d2f22af12bcc3c384b67a542360a6d2ee5 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 16:59:18 +0200
Subject: [PATCH 15/21] vfs_fruit: let the ad_convert_*() subfunctions mmap as
 needed

This may mean that we mmap twice when we convert an AppleDouble file,
but this is the only sane way to cleanly modularize ad_convert().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 98 ++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 36 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index aaaf5fde97e..f72bec153b2 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -943,13 +943,16 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 }
 
 static bool ad_convert_xattr(struct adouble *ad,
-			     const struct smb_filename *smb_fname,
-			     char *map)
+			     const struct smb_filename *smb_fname)
 {
 	static struct char_mappings **string_replace_cmaps = NULL;
+	char *map = MAP_FAILED;
+	size_t maplen;
 	uint16_t i;
 	int saved_errno = 0;
 	NTSTATUS status;
+	int rc;
+	bool ok;
 
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
@@ -967,6 +970,17 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mappings);
 	}
 
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
 	for (i = 0; i < ad->adx_header.adx_num_attrs; i++) {
 		struct ad_xattr_entry *e = &ad->adx_entries[i];
 		char *mapped_name = NULL;
@@ -985,14 +999,16 @@ static bool ad_convert_xattr(struct adouble *ad,
 		    !NT_STATUS_EQUAL(status, NT_STATUS_NONE_MAPPED))
 		{
 			DBG_ERR("string_replace_allocate failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		tmp = mapped_name;
 		mapped_name = talloc_asprintf(talloc_tos(), ":%s", tmp);
 		TALLOC_FREE(tmp);
 		if (mapped_name == NULL) {
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		stream_name = synthetic_smb_fname(talloc_tos(),
@@ -1003,7 +1019,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(mapped_name);
 		if (stream_name == NULL) {
 			DBG_ERR("synthetic_smb_fname failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		DBG_DEBUG("stream_name: %s\n", smb_fname_str_dbg(stream_name));
@@ -1030,7 +1047,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 		TALLOC_FREE(stream_name);
 		if (!NT_STATUS_IS_OK(status)) {
 			DBG_ERR("SMB_VFS_CREATE_FILE failed\n");
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		nwritten = SMB_VFS_PWRITE(fsp,
@@ -1042,18 +1060,29 @@ static bool ad_convert_xattr(struct adouble *ad,
 			saved_errno = errno;
 			close_file(NULL, fsp, ERROR_CLOSE);
 			errno = saved_errno;
-			return false;
+			ok = false;
+			goto fail;
 		}
 
 		status = close_file(NULL, fsp, NORMAL_CLOSE);
 		if (!NT_STATUS_IS_OK(status)) {
-			return false;
+			ok = false;
+			goto fail;
 		}
 		fsp = NULL;
 	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
-	return true;
+	ok = true;
+
+fail:
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	return ok;
 }
 
 static bool ad_convert_finderinfo(struct adouble *ad,
@@ -1171,17 +1200,37 @@ static bool ad_convert_truncate(struct adouble *ad,
 }
 
 static bool ad_convert_move_reso(struct adouble *ad,
-				 const struct smb_filename *smb_fname,
-				 char *map)
+				 const struct smb_filename *smb_fname)
 {
+	char *map = MAP_FAILED;
+	size_t maplen;
+	int rc;
+
 	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
 		return true;
 	}
 
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
 	memmove(map + ADEDOFF_RFORK_DOT_UND,
 		map + ad_getentryoff(ad, ADEID_RFORK),
 		ad_getentrylen(ad, ADEID_RFORK));
 
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
 	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
 	return true;
@@ -1199,9 +1248,6 @@ static bool ad_convert_move_reso(struct adouble *ad,
 static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
-	int rc = 0;
-	char *map = MAP_FAILED;
-	size_t origlen;
 	ssize_t len;
 	bool ok;
 
@@ -1209,38 +1255,18 @@ static int ad_convert(struct adouble *ad,
 		return 0;
 	}
 
-	origlen = ad_getentryoff(ad, ADEID_RFORK) +
-		ad_getentrylen(ad, ADEID_RFORK);
-
-	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, origlen, PROT_READ|PROT_WRITE, MAP_SHARED,
-		   ad->ad_fd, 0);
-	if (map == MAP_FAILED) {
-		DEBUG(2, ("mmap AppleDouble: %s\n", strerror(errno)));
-		return -1;
-	}
-
-	ok = ad_convert_xattr(ad, smb_fname, map);
+	ok = ad_convert_xattr(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
 		return -1;
 	}
 
-	ok = ad_convert_move_reso(ad, smb_fname, map);
+	ok = ad_convert_move_reso(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
 		return -1;
 	}
 
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
-		munmap(map, origlen);
-		return -1;
-	}
-
-	rc = munmap(map, origlen);
-	if (rc != 0) {
-		DBG_ERR("munmap failed: %s\n", strerror(errno));
 		return -1;
 	}
 
-- 
2.13.6


From 8808bc48402c6852427af6858cf4cebc5122d485 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 17:07:45 +0200
Subject: [PATCH 16/21] vfs_fruit: let the ad_convert_*() subfunction update
 the on-disk AppleDoube header as needed

Another step in simplifying ad_convert() itself. It means that we may
write to disk twice, but is only ever done once per AppleDouble file.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index f72bec153b2..c76dcb38671 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -949,6 +949,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 	char *map = MAP_FAILED;
 	size_t maplen;
 	uint16_t i;
+	ssize_t len;
 	int saved_errno = 0;
 	NTSTATUS status;
 	int rc;
@@ -1073,6 +1074,20 @@ static bool ad_convert_xattr(struct adouble *ad,
 	}
 
 	ad_setentrylen(ad, ADEID_FINDERI, ADEDLEN_FINDERI);
+
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		goto fail;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		ok = false;
+		goto fail;
+	}
+
 	ok = true;
 
 fail:
@@ -1204,7 +1219,9 @@ static bool ad_convert_move_reso(struct adouble *ad,
 {
 	char *map = MAP_FAILED;
 	size_t maplen;
+	ssize_t len;
 	int rc;
+	bool ok;
 
 	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
 		return true;
@@ -1233,6 +1250,18 @@ static bool ad_convert_move_reso(struct adouble *ad,
 
 	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
 
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return false;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return false;
+	}
+
 	return true;
 }
 
@@ -1248,7 +1277,6 @@ static bool ad_convert_move_reso(struct adouble *ad,
 static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
-	ssize_t len;
 	bool ok;
 
 	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
@@ -1270,18 +1298,6 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return -1;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return -1;
-	}
-
 	ok = ad_convert_finderinfo(ad, smb_fname);
 	if (!ok) {
 		DBG_ERR("Failed to convert [%s]\n",
-- 
2.13.6


From 33657f076bfaec77c08678188a65b4b24ea98660 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Fri, 5 Oct 2018 22:05:43 +0200
Subject: [PATCH 17/21] vfs_fruit: call ad_convert_move_reso() from
 ad_convert_xattr()

ad_convert_xattr() is the place that triggers the need to move the
resource fork, so it should also call ad_convert_move_reso().

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 113 ++++++++++++++++++++++----------------------
 1 file changed, 57 insertions(+), 56 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index c76dcb38671..f7cba8a0728 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -942,6 +942,58 @@ static bool ad_unpack(struct adouble *ad, const size_t nentries,
 	return true;
 }
 
+static bool ad_convert_move_reso(struct adouble *ad,
+				 const struct smb_filename *smb_fname)
+{
+	char *map = MAP_FAILED;
+	size_t maplen;
+	ssize_t len;
+	int rc;
+	bool ok;
+
+	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
+		return true;
+	}
+
+	maplen = ad_getentryoff(ad, ADEID_RFORK) +
+		ad_getentrylen(ad, ADEID_RFORK);
+
+	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
+	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
+		   ad->ad_fd, 0);
+	if (map == MAP_FAILED) {
+		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
+		return false;
+	}
+
+
+	memmove(map + ADEDOFF_RFORK_DOT_UND,
+		map + ad_getentryoff(ad, ADEID_RFORK),
+		ad_getentrylen(ad, ADEID_RFORK));
+
+	rc = munmap(map, maplen);
+	if (rc != 0) {
+		DBG_ERR("munmap failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
+
+	ok = ad_pack(ad);
+	if (!ok) {
+		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
+		return false;
+	}
+
+	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
+	if (len != AD_DATASZ_DOT_UND) {
+		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
+		return false;
+	}
+
+	return true;
+}
+
 static bool ad_convert_xattr(struct adouble *ad,
 			     const struct smb_filename *smb_fname)
 {
@@ -1088,6 +1140,11 @@ static bool ad_convert_xattr(struct adouble *ad,
 		goto fail;
 	}
 
+	ok = ad_convert_move_reso(ad, smb_fname);
+	if (!ok) {
+		goto fail;
+	}
+
 	ok = true;
 
 fail:
@@ -1214,57 +1271,6 @@ static bool ad_convert_truncate(struct adouble *ad,
 	return true;
 }
 
-static bool ad_convert_move_reso(struct adouble *ad,
-				 const struct smb_filename *smb_fname)
-{
-	char *map = MAP_FAILED;
-	size_t maplen;
-	ssize_t len;
-	int rc;
-	bool ok;
-
-	if (ad_getentrylen(ad, ADEID_RFORK) == 0) {
-		return true;
-	}
-
-	maplen = ad_getentryoff(ad, ADEID_RFORK) +
-		ad_getentrylen(ad, ADEID_RFORK);
-
-	/* FIXME: direct use of mmap(), vfs_aio_fork does it too */
-	map = mmap(NULL, maplen, PROT_READ|PROT_WRITE, MAP_SHARED,
-		   ad->ad_fd, 0);
-	if (map == MAP_FAILED) {
-		DBG_ERR("mmap AppleDouble: %s\n", strerror(errno));
-		return false;
-	}
-
-	memmove(map + ADEDOFF_RFORK_DOT_UND,
-		map + ad_getentryoff(ad, ADEID_RFORK),
-		ad_getentrylen(ad, ADEID_RFORK));
-
-	rc = munmap(map, maplen);
-	if (rc != 0) {
-		DBG_ERR("munmap failed: %s\n", strerror(errno));
-		return false;
-	}
-
-	ad_setentryoff(ad, ADEID_RFORK, ADEDOFF_RFORK_DOT_UND);
-
-	ok = ad_pack(ad);
-	if (!ok) {
-		DBG_WARNING("ad_pack [%s] failed\n", smb_fname->base_name);
-		return false;
-	}
-
-	len = sys_pwrite(ad->ad_fd, ad->ad_data, AD_DATASZ_DOT_UND, 0);
-	if (len != AD_DATASZ_DOT_UND) {
-		DBG_ERR("%s: bad size: %zd\n", smb_fname->base_name, len);
-		return false;
-	}
-
-	return true;
-}
-
 /**
  * Convert from Apple's ._ file to Netatalk
  *
@@ -1288,11 +1294,6 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_convert_move_reso(ad, smb_fname);
-	if (!ok) {
-		return -1;
-	}
-
 	ok = ad_convert_truncate(ad, smb_fname);
 	if (!ok) {
 		return -1;
-- 
2.13.6


From 0a606ddcf3d6ae53525caf160e68ee7abef28e27 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 12:51:37 +0200
Subject: [PATCH 18/21] vfs_fruit: add check for OS X filler in FinderInfo
 conversion

This ensures that the function only acts on AppleDouble files created by
macOS and not AppleDouble files created by us that are already in the
correct format (only using the Resource Fork).

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index f7cba8a0728..9987f354d2b 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1169,6 +1169,12 @@ static bool ad_convert_finderinfo(struct adouble *ad,
 	ssize_t nwritten;
 	NTSTATUS status;
 	int saved_errno = 0;
+	int cmp;
+
+	cmp = memcmp(ad->ad_filler, AD_FILLER_TAG_OSX, ADEDLEN_FILLER);
+	if (cmp != 0) {
+		return true;
+	}
 
 	p_ad = ad_get_entry(ad, ADEID_FINDERI);
 	if (p_ad == NULL) {
-- 
2.13.6


From f01058cae0a122dabeea79916b8e350b541bbcff Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 18:43:51 +0200
Subject: [PATCH 19/21] vfs_fruit: add out arg "converted_xattr" to
 ad_convert_xattr

Used to let the caller know if a conversion has been done. Currently not
used in the caller, that comes next.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 9987f354d2b..3b491aff301 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -995,7 +995,8 @@ static bool ad_convert_move_reso(struct adouble *ad,
 }
 
 static bool ad_convert_xattr(struct adouble *ad,
-			     const struct smb_filename *smb_fname)
+			     const struct smb_filename *smb_fname,
+			     bool *converted_xattr)
 {
 	static struct char_mappings **string_replace_cmaps = NULL;
 	char *map = MAP_FAILED;
@@ -1007,6 +1008,8 @@ static bool ad_convert_xattr(struct adouble *ad,
 	int rc;
 	bool ok;
 
+	*converted_xattr = false;
+
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
 	}
@@ -1145,6 +1148,7 @@ static bool ad_convert_xattr(struct adouble *ad,
 		goto fail;
 	}
 
+	*converted_xattr = true;
 	ok = true;
 
 fail:
@@ -1290,12 +1294,13 @@ static int ad_convert(struct adouble *ad,
 		      const struct smb_filename *smb_fname)
 {
 	bool ok;
+	bool converted_xattr;
 
 	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
 		return 0;
 	}
 
-	ok = ad_convert_xattr(ad, smb_fname);
+	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
 	if (!ok) {
 		return -1;
 	}
-- 
2.13.6


From 2f350ce6e82facdfed420fbc1c89ef438ddcdb40 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 8 Oct 2018 18:47:32 +0200
Subject: [PATCH 20/21] vfs_fruit: make call to ad_convert_truncate() optional

Call ad_convert_truncate() based on whether the previous call
ad_convert_xattr() returned converted_xattr=true.

Upcoming fixes for a different Samba bug (#13642) will hook into calling
ad_convert_truncate() in other cases, this also prepares for that.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 source3/modules/vfs_fruit.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 3b491aff301..c16cad0f1c3 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1305,9 +1305,11 @@ static int ad_convert(struct adouble *ad,
 		return -1;
 	}
 
-	ok = ad_convert_truncate(ad, smb_fname);
-	if (!ok) {
-		return -1;
+	if (converted_xattr) {
+		ok = ad_convert_truncate(ad, smb_fname);
+		if (!ok) {
+			return -1;
+		}
 	}
 
 	ok = ad_convert_finderinfo(ad, smb_fname);
-- 
2.13.6


From c53ac91e7628a2068875e03627365c0411a19ffb Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 9 Oct 2018 10:15:37 +0200
Subject: [PATCH 21/21] vfs_fruit: move check in ad_convert() to ad_convert_*()
 subfunctions

Currently the whole conversion is skipped if the FinderInfo entry in the
AppleDouble file is of the default size (ie not containing xattrs).

That also means we never converted FinderInfo from the AppleDouble file
to stream format. This change finally fixes this.

Note that this keeps failing with streams_depot, much like the existing
known-fail of "samba3.vfs.fruit streams_depot.OS X AppleDouble file
conversion". Fixing the conversion to work with vfs_streams_depot is a
task for another day.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13649

Signed-off-by: Ralph Boehme <slow at samba.org>
---
 selftest/knownfail.d/samba3.vfs.fruit | 2 --
 source3/modules/vfs_fruit.c           | 8 ++++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/selftest/knownfail.d/samba3.vfs.fruit b/selftest/knownfail.d/samba3.vfs.fruit
index bc46c2f4922..6307e2b3404 100644
--- a/selftest/knownfail.d/samba3.vfs.fruit
+++ b/selftest/knownfail.d/samba3.vfs.fruit
@@ -1,4 +1,2 @@
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion\(nt4_dc\)
-^samba3.vfs.fruit metadata_netatalk.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
-^samba3.vfs.fruit metadata_stream.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
 ^samba3.vfs.fruit streams_depot.OS X AppleDouble file conversion without embedded xattr\(nt4_dc\)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index c16cad0f1c3..4420cb6021e 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -1010,6 +1010,10 @@ static bool ad_convert_xattr(struct adouble *ad,
 
 	*converted_xattr = false;
 
+	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
+		return true;
+	}
+
 	if (ad->adx_header.adx_num_attrs == 0) {
 		return true;
 	}
@@ -1296,10 +1300,6 @@ static int ad_convert(struct adouble *ad,
 	bool ok;
 	bool converted_xattr;
 
-	if (ad_getentrylen(ad, ADEID_FINDERI) == ADEDLEN_FINDERI) {
-		return 0;
-	}
-
 	ok = ad_convert_xattr(ad, smb_fname, &converted_xattr);
 	if (!ok) {
 		return -1;
-- 
2.13.6



More information about the samba-technical mailing list