From owner-freebsd-current@freebsd.org Mon Oct 22 21:24:08 2018 Return-Path: Delivered-To: freebsd-current@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 03A9FFD73B3 for ; Mon, 22 Oct 2018 21:24:08 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from CAN01-TO1-obe.outbound.protection.outlook.com (mail-eopbgr670087.outbound.protection.outlook.com [40.107.67.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (Client CN "mail.protection.outlook.com", Issuer "GlobalSign Organization Validation CA - SHA256 - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 978BA74B42; Mon, 22 Oct 2018 21:24:07 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM (52.132.50.155) by YTOSPR01MB018.CANPRD01.PROD.OUTLOOK.COM (52.132.51.11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1250.30; Mon, 22 Oct 2018 21:24:06 +0000 Received: from YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM ([fe80::b09c:c689:9517:ea2b]) by YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM ([fe80::b09c:c689:9517:ea2b%4]) with mapi id 15.20.1250.028; Mon, 22 Oct 2018 21:24:06 +0000 From: Rick Macklem To: Brooks Davis CC: FreeBSD Current , Josh Paetzel Subject: Re: which way to update export_args structure? Thread-Topic: which way to update export_args structure? Thread-Index: AQHUWq0WJEV5v0B9NEODb8Fv+fAB/qUNrJCAgAZGIdGAAanmAIAR0AV4gAQgCwCAAFONFg== Date: Mon, 22 Oct 2018 21:24:06 +0000 Message-ID: References: <20181003155133.GA57729@spindle.one-eyed-alien.net> <20181008170428.GB9766@spindle.one-eyed-alien.net> , <20181022160508.GB45769@spindle.one-eyed-alien.net> In-Reply-To: <20181022160508.GB45769@spindle.one-eyed-alien.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=rmacklem@uoguelph.ca; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; YTOSPR01MB018; 6:3qQdqH1quYIykgqanG+0CgGLzppQncCCvZOMqz3fKuakFm0oVAcCb9Yfdxy9qOvWX2BYdic6w/AyCAWQy0qSYxvLvPxOvb3CK7y7AfcdOLrhbKVU6TPZZ6ZUMu1PLcJRinKX/G/YbkvazIPWb8n0LQCbGeHvJxQOr54RMdkX35952zsbuNfjFhGPsFW+3ztsSVmG1bPcEPsaPzId/egTaIv0HO5eOKjjSVxl4Qd/V0I1r1xb05Vy/3h+wN4rYZ5orWHMTRZwb9nMCIyoRENPdmgBohw2k59zPzho3ErtIpbAB9dWtWAndMslwf5+KNZfB9GBNTSSVQ4XURATIApkB/hVO2Nbp6C1btP5pxRqPzThOBusIeAtYmgFJ6sRSgf8WwvUMjaBB3Iz3SUJXtQGYZ4ut3yRyorxTcD1C8zCdI1EdbjoFx6AcxIJ1XBHZk3/gzxghdSJMlneT36W+A313w==; 5:qmQHRCxwk3M6q63IMPTkTva0i0wvc0x4C1vbQB1CgqE0UnkAxJ80RDJ2Oxa+c8jXEkjLpZ5heyEBs5Z1IBC3qM5UE73FgRYf7h2VaOjP1gVIXWfJ8+AY1b+YTFgNv8rrDqbQtFaU5XcOVd0+NupLby3dq65dha3gMLEjDM7OGZY=; 7:Zt8NEH8fRSnMu9p3uD+oLtO9cdy0IFm+rQ0EUJGbmHmkxvTQ/TZHM3fc75IEJq/zfmVfsJqnqLXaxUapmX49hyGSP0/dInyHd8dIXBHZ39sfXrjGdex9/CRH900/FM08XxBK0Gr0ORwLpa8EYUAeEkSejbp1kaYNSVOdgYQge5u2/Tm7YFds4k0T2dkYzBhskISOQAQEudIrteY57kWQGgkcBeNaQyIbV068U6/TNRFOFc7GYcCIqMknqTLIFa8z x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 1549bd33-ebcb-441c-8964-08d63864b289 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(2017052603328)(7153060)(7193020); SRVR:YTOSPR01MB018; x-ms-traffictypediagnostic: YTOSPR01MB018: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040522)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3231355)(944501410)(52105095)(3002001)(148016)(149066)(150057)(6041310)(20161123560045)(20161123564045)(20161123558120)(20161123562045)(201703131423095)(201702281529075)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:YTOSPR01MB018; BCL:0; PCL:0; RULEID:; SRVR:YTOSPR01MB018; x-forefront-prvs: 08331F819E x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(396003)(136003)(346002)(366004)(39860400002)(376002)(199004)(189003)(106356001)(76176011)(11346002)(7696005)(476003)(446003)(2906002)(5250100002)(14454004)(33656002)(74316002)(8936002)(186003)(81166006)(105586002)(14444005)(81156014)(97736004)(256004)(54906003)(46003)(55016002)(305945005)(6916009)(9686003)(786003)(6436002)(316002)(25786009)(71190400001)(8676002)(71200400001)(229853002)(102836004)(93886005)(478600001)(6506007)(6246003)(86362001)(99286004)(74482002)(5660300001)(53936002)(486006)(2900100001)(68736007)(4326008)(43043002); DIR:OUT; SFP:1101; SCL:1; SRVR:YTOSPR01MB018; H:YTOPR0101MB1162.CANPRD01.PROD.OUTLOOK.COM; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: uoguelph.ca does not designate permitted sender hosts) x-microsoft-antispam-message-info: 9hcY7WVZaCRgcIZuq8aoWigXC0o6xDquUeFqpomUsBm+OYG4aOUOqPsZGFO4TnuDqDLrnq+ySae/6JQKFbXllFA7mrlxioD8cq+lREwYY7jGpC3JfNZZICzCmVC53FPL2TrVv0FcGto35hIMvtnOJ1bJDJut1TTciNC5xKIbLuzqt5+X6pUxa190sSAmNM397Tri4o3WmZ64L0Y226y0X2rm30QDrpbTR/EfpmxF1JnDcvPQ/iEwWKHhMKdVmmkKPSgjSp44fRx2pDAXleFNRjkSaLtVWEJ6BM062QyI+rNu/AtupM0YOPkD7pjBPtmmuPD/jmMMR9LC1AmNagSp3212J91NiStB7HcF5HBE/vQ= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: uoguelph.ca X-MS-Exchange-CrossTenant-Network-Message-Id: 1549bd33-ebcb-441c-8964-08d63864b289 X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Oct 2018 21:24:06.2278 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: be62a12b-2cad-49a1-a5fa-85f4f3156a7d X-MS-Exchange-Transport-CrossTenantHeadersStamped: YTOSPR01MB018 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Oct 2018 21:24:08 -0000 Brooks Davis wrote: On Sat, Oct 20, 2018 at 01:17:37AM +0000, Rick Macklem wrote: [lots of stuff snipped] >> + if (error =3D=3D 0) { >> + gotexp =3D 0; >> + memset(&export, 0, sizeof(export)); >> + if (vfs_getopt(mp->mnt_optnew, "export.exflags", &bufp, >> + &len) =3D=3D 0) { >> + gotexp =3D 1; >> + export.ex_flags =3D strtouq(bufp, &endptr, 0); >> + if (endptr =3D=3D bufp) >> + export_error =3D EINVAL; >> + } > >This pattern involving strtouq seems wrong to me. We should probably >pass a pointer to the integer type (which should always be an explicitly >sized type). Thanks for noticing this. I was "on the fence" w.r.t. whether the arguments= should all be architecture neutral (ascii representation or ???). (I didn't bother doing the code for the net addresses in ascii notation.) It looks like you don't see that as needed, so I will just pass the binary = numbers in. (We could put them in Big Endian order, if someone thinks that is necessary= ?) >If it didn't contradict the first sentence of the description in >vfs_getopt.9, I'd say we should pass int and long values in the length >with a NULL buffer. Heh, heh. That would be a sneaky hack. I like it, but will resist and use t= he bufp. [more stuff snipped] >> + if (vfs_getopt(mp->mnt_optnew, "export", &bufp, &len) =3D= =3D 0) { >> + /* Assume that there is only 1 ABI for each length= . */ >> + switch (len) { >> + case (sizeof(struct oexport_args)): >> + case (sizeof(struct o2export_args)): >> + memset(&export, 0, sizeof(export)); > >I think this is now redundant. I'll be looking at the code to-day, but I think there is at least one field (ex_suppgroups) that needs to be set NULL so that the free() won't blow up. (I tend to bzero()/memset(0) so that any future additional fields don't hav= e to be explicitly set 0/NULL in the compatibility code, but it there aren't curren= tly any, I may take this out. This code doesn't get executed frequently, so efficien= cy at the "save a few instructions" level doesn't matter, imho.) >> + memset(&o2export, 0, sizeof(o2export)); > >This is certainly redundant given that you immediately copy over it. Yep. This was just cruft left over from a previous version of the patch. >This is the direction I'd been thinking. FWIW, the usecase is more that >once you've moved away from the struct it's easy to make incremental >changes then to use a 32-bit mountd on a 64-bit kernel. Moving toward >size-independent interfaces helps both causes though. Cool. I'll finish the patch to-day and then let Josh work on mountd.c. Thanks for your comments, rick