Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Corey Bryant <coreyb@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