Date: Sat, 19 Dec 2015 21:58:23 +0100 From: Ralf Wenk <iz-rpi03@hs-karlsruhe.de> To: Ian Lepore <ian@freebsd.org> Cc: freebsd-arm <freebsd-arm@freebsd.org>, Rui Paulo <rpaulo@me.com>, John-Mark Gurney <jmg@funkthat.com>, Warner Losh <imp@bsdimp.com> Subject: Re: mount_smbfs Message-ID: <E1aAOa8-007Wk6-Mg@smtp.hs-karlsruhe.de> In-Reply-To: <1450487583.25138.132.camel@freebsd.org> References: <54B9DCD1.3040306@foxvalley.net> <4759EAA0-D4AA-4923-9350-B7E753819169@me.com> <6E32991C3BD8465DB8DB0E65DFDA47AA@ad.peach.ne.jp> <20150123195403.GO1949@funkthat.com> <ED004DCD-35B9-4F2E-9F5D-41BF33E66F01@bsdimp.com> <20150123213619.GP1949@funkthat.com> <1450487583.25138.132.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Ian Lepore wrote on Fri, 18 Dec 2015 at 18:13:03 -0700:
> On Fri, 2015-01-23 at 13:36 -0800, John-Mark Gurney wrote:
> > Warner Losh wrote this message on Fri, Jan 23, 2015 at 13:21 -0800:
> > >
> > > > On Jan 23, 2015, at 11:54 AM, John-Mark Gurney <jmg@funkthat.com> wrote:
> > > >
> > > > Daisuke Aoyama wrote this message on Sat, Jan 24, 2015 at 03:07 +0900:
> > > > > Quick hack patch is attached.
> > > >
> > > > Please use {l,b}e16dec, or if the code is suppose to be native endian,
> > > > make it dependant on __NO_STRICT_ALIGNMENT and add the proper endian
> > > > swap, not __arm__ as there are other arches that require the same fix...
> > >
> > > If there???s just a couple of places that need this, don???t bother making them dependent
> > > on __NO_STRICT_ALIGNMENT. That clutters things up a bit too much. Given the 3
> > > or 4 places this is used, and the relative infrequency of the calls, just doing a memcpy
> > > unconditionally is always correct and reduces the risk of one branch of the #if being
> > > changed w/o the other. Since it is already using NBENCODE(), I think that using
> > > {l,b}e16enc (not dec) would be a larger code churn.
> >
> > Clearly neither of us looked at the code closely... NBENCODE should be
> > rewritten to take a pointer and use le16enc... Then memsetw should just
> > call NBENCODE internally as it goes...
>
> Well I looked at the code closely, even if it did take me almost a year
> to get around to it. :)
>
> The conclusion I reached is that alignment and endian problems should
> just be turned into a non-issue by discarding the existing macro and
> memsetw() function and writing a new inline function that deals purely
> with bytes instead of 16-bit values.
>
> It's up for review on phabricator at https://reviews.freebsd.org/D4622
>
> And I'll also attach the diff to this mail for anyone who wants to test
> it (which I can't do, no smb server).
Done. Your patch applied to revision 292468 is working here nice.
The PR https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=180438
could be added to the phabricator information as well.
Thank you.
Ralf
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E1aAOa8-007Wk6-Mg>