From owner-freebsd-arm@freebsd.org Sat Dec 19 01:13:06 2015 Return-Path: Delivered-To: freebsd-arm@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id AB96EA4CF33 for ; Sat, 19 Dec 2015 01:13:06 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from erouter6.ore.mailhop.org (erouter6.ore.mailhop.org [54.187.213.119]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 91E3E1AFA for ; Sat, 19 Dec 2015 01:13:06 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from ilsoft.org (unknown [73.34.117.227]) by outbound3.ore.mailhop.org (Halon Mail Gateway) with ESMTPSA; Sat, 19 Dec 2015 01:12:50 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.14.9/8.14.9) with ESMTP id tBJ1D3MG028599; Fri, 18 Dec 2015 18:13:03 -0700 (MST) (envelope-from ian@freebsd.org) Message-ID: <1450487583.25138.132.camel@freebsd.org> Subject: Re: mount_smbfs From: Ian Lepore To: John-Mark Gurney , Warner Losh Cc: freebsd-arm , Rui Paulo Date: Fri, 18 Dec 2015 18:13:03 -0700 In-Reply-To: <20150123213619.GP1949@funkthat.com> References: <54B9DCD1.3040306@foxvalley.net> <4759EAA0-D4AA-4923-9350-B7E753819169@me.com> <6E32991C3BD8465DB8DB0E65DFDA47AA@ad.peach.ne.jp> <20150123195403.GO1949@funkthat.com> <20150123213619.GP1949@funkthat.com> Content-Type: multipart/mixed; boundary="=-csbj0LHgt/uWaSTdnj7N" X-Mailer: Evolution 3.16.5 FreeBSD GNOME Team Port Mime-Version: 1.0 X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "Porting FreeBSD to ARM processors." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 19 Dec 2015 01:13:06 -0000 --=-csbj0LHgt/uWaSTdnj7N Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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 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). -- Ian --=-csbj0LHgt/uWaSTdnj7N Content-Disposition: inline; filename="smbfs_nb_name.diff" Content-Type: text/x-patch; name="smbfs_nb_name.diff"; charset="us-ascii" Content-Transfer-Encoding: 7bit Index: contrib/smbfs/lib/smb/nb_name.c =================================================================== --- contrib/smbfs/lib/smb/nb_name.c (revision 292290) +++ contrib/smbfs/lib/smb/nb_name.c (working copy) @@ -143,15 +143,13 @@ nb_encname_len(const char *str) return len; } -#define NBENCODE(c) (htole16((u_short)(((u_char)(c) >> 4) | \ - (((u_char)(c) & 0xf) << 8)) + 0x4141)) +static inline void +nb_char_encode(u_char **ptr, u_char c, int n) +{ -static void -memsetw(char *dst, int n, u_short word) -{ while (n--) { - *(u_short*)dst = word; - dst += 2; + *(*ptr)++ = 0x41 + (c >> 4); + *(*ptr)++ = 0x41 + (c & 0x0f); } } @@ -165,19 +163,15 @@ nb_name_encode(struct nb_name *np, u_char *dst) *cp++ = NB_ENCNAMELEN; name = np->nn_name; if (name[0] == '*' && name[1] == 0) { - *(u_short*)cp = NBENCODE('*'); - memsetw(cp + 2, NB_NAMELEN - 1, NBENCODE(' ')); - cp += NB_ENCNAMELEN; + nb_char_encode(&cp, '*', 1); + nb_char_encode(&cp, ' ', NB_NAMELEN - 1); } else { - for (i = 0; *name && i < NB_NAMELEN - 1; i++, cp += 2, name++) - *(u_short*)cp = NBENCODE(toupper(*name)); - i = NB_NAMELEN - i - 1; - if (i > 0) { - memsetw(cp, i, NBENCODE(' ')); - cp += i * 2; - } - *(u_short*)cp = NBENCODE(np->nn_type); - cp += 2; + for (i = 0; i < NB_NAMELEN - 1; i++) + if (*name != 0) + nb_char_encode(&cp, toupper(*name++), 1); + else + nb_char_encode(&cp, ' ', 1); + nb_char_encode(&cp, np->nn_type, 1); } *cp = 0; if (np->nn_scope == NULL) --=-csbj0LHgt/uWaSTdnj7N--