From owner-freebsd-arch@FreeBSD.ORG Tue Apr 13 19:01:27 2010 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EBD371065679; Tue, 13 Apr 2010 19:01:27 +0000 (UTC) (envelope-from delphij@delphij.net) Received: from tarsier.geekcn.org (tarsier.geekcn.org [IPv6:2001:470:a803::1]) by mx1.freebsd.org (Postfix) with ESMTP id BE1B48FC24; Tue, 13 Apr 2010 19:01:26 +0000 (UTC) Received: from mail.geekcn.org (tarsier.geekcn.org [211.166.10.233]) by tarsier.geekcn.org (Postfix) with ESMTP id 58CDAA56587; Wed, 14 Apr 2010 03:01:25 +0800 (CST) X-Virus-Scanned: amavisd-new at geekcn.org Received: from tarsier.geekcn.org ([211.166.10.233]) by mail.geekcn.org (mail.geekcn.org [211.166.10.233]) (amavisd-new, port 10024) with LMTP id KkbYGbmQsBul; Wed, 14 Apr 2010 03:01:19 +0800 (CST) Received: from delta.delphij.net (drawbridge.ixsystems.com [206.40.55.65]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by tarsier.geekcn.org (Postfix) with ESMTPSA id 41B2FA5502B; Wed, 14 Apr 2010 03:01:18 +0800 (CST) DomainKey-Signature: a=rsa-sha1; s=default; d=delphij.net; c=nofws; q=dns; h=message-id:date:from:reply-to:organization:user-agent: mime-version:to:cc:subject:references:in-reply-to: x-enigmail-version:openpgp:content-type:content-transfer-encoding; b=L/qffVBjyhfSQzVWULPMzhlpceKJ99kIViVTjOxzdbOUC+sOISvr4ieVvryH8f92g KxRRtl/bkuW7JMCpG7G1w== Message-ID: <4BC4BF7A.9090106@delphij.net> Date: Tue, 13 Apr 2010 12:01:14 -0700 From: Xin LI Organization: The Geek China Organization User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.1.9) Gecko/20100408 Thunderbird/3.0.4 ThunderBrowse/3.2.8.1 MIME-Version: 1.0 To: John Baldwin References: <4BC39E93.7060906@delphij.net> <20100412233330.GC19003@elvis.mu.org> <4BC3BA48.9010009@delphij.net> <201004130853.16994.jhb@freebsd.org> In-Reply-To: <201004130853.16994.jhb@freebsd.org> X-Enigmail-Version: 1.0.1 OpenPGP: id=3FCA37C1; url=http://www.delphij.net/delphij.asc Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Alfred Perlstein , d@delphij.net, freebsd-arch@freebsd.org Subject: Re: _IOWR when errno != 0 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: d@delphij.net List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Apr 2010 19:01:28 -0000 -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 2010/04/13 05:53, John Baldwin wrote: > On Monday 12 April 2010 8:26:48 pm Xin LI wrote: >> On 2010/04/12 16:33, Alfred Perlstein wrote: >>> * Xin LI [100412 15:28] wrote: >>>> -----BEGIN PGP SIGNED MESSAGE----- >>>> Hash: SHA1 >>>> >>>> Hi, >>>> >>>> Is there a sane way to copyout ioctl request when the returning errno != >>>> 0? Looking at the code, currently, in sys/kern/sys_generic.c, we have: >>>> >>>> =========== >>>> error = kern_ioctl(td, uap->fd, com, data); >>>> >>>> if (error == 0 && (com & IOC_OUT)) >>>> error = copyout(data, uap->data, (u_int)size); >>>> =========== >>>> >>>> Is there any objection if I change it to something like: >>>> >>>> =========== >>>> saved_error = kern_ioctl(td, uap->fd, com, data); >>>> >>>> if (com & IOC_OUT) >>>> error = copyout(data, uap->data, (u_int)size); >>>> if (saved_error) >>>> error = saved_error; >>>> =========== >>> >>> Is this for linux compat? >> >> Do they do this way? I'm not quite sure :-/ >> >> I got a bug report and am thinking about how to fix it, it seems that we >> do not have a generic way of returning an error number while giving some >> "hints" about the error at the same time, for the ioctl() call. Adding >> an extra pointer to the request structure seems to be a last-resort way >> and sounds to be ugly. > > Actually, this pattern of embedding an error is quite common. The mfi(4) and > mpt(4) pass-thru ioctls to send firmware commands embed the return status of > any firmware command in the structure that is passed in and out for example. > >>> I'm not sure this would work, it might seriously break userland >>> compat. Have you looked around/queiried what the expected outcome >>> is from a bad ioctl? By default the buffer will be zero'd this >>> might be unexpected by apps. (all or nothing) >> >> Yes that's exactly why I'm asking, my understanding is that for normal >> usages would be something like: >> >> if (ioctl(fd, SIOCSOMETHING, &req) < 0) { >> // do something to handle the error >> } else { >> // use data fed back from req >> } >> >> In this case, I think the result would not be affected. Is there many >> (if any) programs that don't bother to check return value of ioctl()? >> >> Speaking for the userland buffer, for _IOR ioctls, the side effect would >> be that userland would see a zeroed out 'req' structure (kernel buffer >> gets zeroed out before calling kern_ioctl), or "half-baked" one (the >> kernel code may have only written partial data). For _IOWR ioctls, the >> side effect would be that the userland may get half-baked data. > > You miss one important variation where the error handling involves adjusting > the request and retrying (or submitting the same request to a different ioctl > to handle renumbering conflicts, etc.). Other APIs such as sysctl(2) and > setsockopt(2) can leave partial data, but the callers of those APIs expect > that (and in fact, those APIs return the actual length of data that is copied > out). ioctl(2) has not had that behavior, however, and I would find it > surprising. I see, that's what I am concerned about, thanks for the explanation. In order to maintain ABI compatibility I now have a patch which changs the current behavior of SIOCGIFDESCR to set the buffer field to NULL and return no errno. The existing code in -HEAD doesn't seem to work when the field is big :( I will post the patch to -net@ for review once I get it tested. Cheers, - -- Xin LI http://www.delphij.net/ FreeBSD - The Power to Serve! Live free or die -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (FreeBSD) iQEcBAEBAgAGBQJLxL96AAoJEATO+BI/yjfBkkMIAIVjUmwrfHLl5F+mIlRD+Zpv hYZVBZaeu3/ymv0Zepo5vhbvJCOWxgdtRnJgoVlkpglZLwVrKkAdfxWp/di5n8xm O4BMc+BIra6tYnqaxmbCYoigKGoLVhim1n6j2Xld/h0n91ErBDpdrWBdHVbs8uV+ mRFLCPbGzGnEXw68rdbWjXFIDRIe7btTdmyYotaHd5AFaqQw6EM+OAXRG3UqGtm3 92o+9TW2LcTTP9gyresbQGoXvITHXVfSdihhDVfDMCtbaClQ+IFlny0oGqg0DttR OhnEWDvBgUQD+aADYx2k8YLXziUsQzvTc7WTZuoxdz3LzZVecyQSewiydEhor/U= =IHjf -----END PGP SIGNATURE-----