deprecated TIOCSPGRP and TIOCGPGRP ioctl command

[Giovanni Trematerra]

On Thu, Nov 10, 2011 at 8:18 PM, Peter Wemm <peter at wemm.org> wrote:
> On Wed, Nov 9, 2011 at 5:52 PM, Garrett Cooper <yanegomi at gmail.com> wrote:
>> On Nov 9, 2011, at 4:49 PM, Peter Wemm wrote:
>>
>>> On Wed, Nov 9, 2011 at 3:39 PM, Giovanni Trematerra <gianni at freebsd.org> wrote:
>>>> Are they deprecated enough to be removed, now?
>>>> FYI FIFO doesn't support them.
>>>>
>>>> --
>>>> Gianni
>>>>
>>>> =================================
>>>> --- sys/kern/sys_pipe.c (revision 227233)
>>>> +++ sys/kern/sys_pipe.c (working copy)
>>>> @@ -1304,17 +1304,6 @@ pipe_ioctl(fp, cmd, data, active_cred, td)
>>>>        *(int *)data = fgetown(&mpipe->pipe_sigio);
>>>>        break;
>>>>
>>>> -   /* This is deprecated, FIOSETOWN should be used instead. */
>>>> -   case TIOCSPGRP:
>>>> -       PIPE_UNLOCK(mpipe);
>>>> -       error = fsetown(-(*(int *)data), &mpipe->pipe_sigio);
>>>> -       goto out_unlocked;
>>>> -
>>>> -   /* This is deprecated, FIOGETOWN should be used instead. */
>>>> -   case TIOCGPGRP:
>>>> -       *(int *)data = -fgetown(&mpipe->pipe_sigio);
>>>> -       break;
>>>> -
>>>>    default:
>>>>        error = ENOTTY;
>>>
>>> Be very very careful with this.  It's part of the classic BSD job
>>> control API.  It would be wise to survey whether any ports shells use
>>> this.
>>>
>>> You might also want to consider things like this in libc:
>>> int
>>> tcsetpgrp(int fd, pid_t pgrp)
>>> {
>>>        int s;
>>>
>>>        s = pgrp;
>>>        return (_ioctl(fd, TIOCSPGRP, &s));
>>> }
>>> Our own libc code uses this, albeit on an API intended to be used on a tty.
>>>
>>> The shell I'd be most concerned about is csh/tcsh in our tree. It has
>>> quite an #ifdef legacy layer and I couldn't convince myself it wasn't
>>> using this indirectly (or the tc* functions) on pipes.
>>>
>>> It might also be an idea to see if the linux compat layer can be
>>> switched over to using the newer API.
>>
>> Move to a compat library perhaps?
>> -Garrett
>
> It's not a compat library candidate.
>
> Summary of the issue:
> ioctl(fd, TIOCSPGRP, arg) can be used on sockets, pipes, ttys.  But not fifos.
> this is a classic BSD job control API.
> libc itself uses this internally on ttys.
> libc doesn't check that the function that does this is actually a tty,
> it could be being used by shells and pipes as an obscure side effect.
> The API is equivalent to FIOSETOWN etc and sockets and pipes use
> common code to implement both ioctl() calls.
>
> The proposed patch was to remove the TIOCSPGRP -> FIOSETOWN mapping
> for pipes only, to sync them with fifos.  ttys and sockets would be
> unaffected.
>
> The implementation function still has to stay.  Its not a large chunk
> of obsolete code that goes away, its just the mapping between the
> ioctl number and the implementation.
>

Thank you Peter, just last question on this.
Having just one ioctl function for both pipe and fifo, would be a problem
having TIOCSPGRP and TIOCGPGRP implemented for fifo?

--
Gianni