[PATCH] Fix AppleDouble conversion in vfs_fruit

Jeremy Allison jra at samba.org
Wed Oct 10 18:20:04 UTC 2018


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 ?

Jeremy.



More information about the samba-technical mailing list