From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39534) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sba9p-0000PV-OQ for qemu-devel@nongnu.org; Mon, 04 Jun 2012 12:29:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sba9n-0004a4-UW for qemu-devel@nongnu.org; Mon, 04 Jun 2012 12:29:29 -0400 Received: from e39.co.us.ibm.com ([32.97.110.160]:44241) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sba9n-0004Zo-Nk for qemu-devel@nongnu.org; Mon, 04 Jun 2012 12:29:27 -0400 Received: from /spool/local by e39.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 4 Jun 2012 10:29:23 -0600 Received: from d01relay04.pok.ibm.com (d01relay04.pok.ibm.com [9.56.227.236]) by d01dlp03.pok.ibm.com (Postfix) with ESMTP id 5C69CC90063 for ; Mon, 4 Jun 2012 12:28:47 -0400 (EDT) Received: from d03av05.boulder.ibm.com (d03av05.boulder.ibm.com [9.17.195.85]) by d01relay04.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q54GSkdN063058 for ; Mon, 4 Jun 2012 12:28:46 -0400 Received: from d03av05.boulder.ibm.com (loopback [127.0.0.1]) by d03av05.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q54GSgVL009662 for ; Mon, 4 Jun 2012 10:28:44 -0600 Message-ID: <4FCCE234.5040109@linux.vnet.ibm.com> Date: Mon, 04 Jun 2012 12:28:36 -0400 From: Corey Bryant MIME-Version: 1.0 References: <1338815410-24890-1-git-send-email-coreyb@linux.vnet.ibm.com> <1338815410-24890-3-git-send-email-coreyb@linux.vnet.ibm.com> <4FCCC6F8.3010901@redhat.com> <4FCCD987.8080205@linux.vnet.ibm.com> <4FCCDC55.5070705@redhat.com> In-Reply-To: <4FCCDC55.5070705@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, stefanha@linux.vnet.ibm.com On 06/04/2012 12:03 PM, Eric Blake wrote: > On 06/04/2012 09:51 AM, Corey Bryant wrote: > >>>> + >>>> + if (strstart(filename, "/dev/fd/",&p)) { >>>> + fd = atoi(p); >>> >>> atoi() is lousy - it has no error checking, and returns 0 if a mistake >>> was made. You really want to be using strtol (or even better, a >>> sensible wrapper around strtol that takes care of the subtleties of >>> calling it correctly), so that you don't end up dup'ing stdin when the >>> user passes a bad /dev/fd/ string. >>> >> >> It looks like strtol returns 0 on failure too. Do we need to support >> stdin/stdout/stderr? > > But at least strtol lets you detect errors: > > char *tmp; > errno = 0; > fd = strtol(p,&tmp, 10); > if (errno || tmp == p) { > /* raise your error here */ > } I don't think this is legitimate. errno can be set under the covers of library calls even if the strtol() call is successful. I was thinking if strtol returns 0 and errno is 0, perhaps we could assume success, but I don't think this is guaranteed either. Maybe a combination of isdigit() then strtol() will give a better idea of success. > > and if you get past that point, then someone really did pass in > /dev/fd/0 as the string they meant to be parsed (probably a user bug, as > getfd is unlikely to ever return 0 unless you start with stdin closed, > which itself is something that POSIX discourages, but not something we > need to specifically worry about). So I would argue that yes, we do > need to support fd 0, if only by not special casing it as compared to > any other valid fd. > Ok -- Regards, Corey