[PATCH] Fix AppleDouble conversion in vfs_fruit

[Ralph Böhme]

On Wed, Oct 10, 2018 at 11:20:04AM -0700, Jeremy Allison wrote:
>On Wed, Oct 10, 2018 at 08:01:25AM +0200, Ralph Böhme wrote:
>> 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.
>
>In [PATCH 19/21] vfs_fruit: add out arg "converted_xattr" to ad_convert_xattr
>
>you have:
>
>@@ -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;
>        }
>
>I think you should initialize converted_xattr
>at declaration as:
>
>+       bool converted_xattr = false;
>
>With that change, RB+. Do you want me to make
>that change and push ?

scaredy-cat band-aid. Pushed with that change. :)))

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