Re: EMACS_GET_TTY_PGRP vs. emacs_get_tty_pgrp

[Dan Nicolaescu]

Ken Brown <address@hidden> writes:

> On 10/20/2010 2:59 PM, Dan Nicolaescu wrote:
>> Ken Brown<address@hidden>  writes:
>>
>>> The macro EMACS_GET_TTY_PGRP defined in src/systty.h uses either
>>> tcgetpgrp or TIOCGPGRP to get the PGID of a terminal's foreground
>>
>> It does not:
>>
>> #ifndef DOS_NT
>> #define EMACS_GET_TTY_PGRP(fd, pgid) (*(pgid) = tcgetpgrp ((fd)))
>> #define EMACS_SET_TTY_PGRP(fd, pgid) (tcsetpgrp ((fd), *(pgid)))
>> #endif /* not DOS_NT */
>>
>> also, I have a patch to remove this, it's only used in two places, so
>> it's not very useful as an abstraction.
>
> Oops.  I looked at the emacs-23 branch but not the trunk.  Sorry for
> the confusion.
>
> But then I still think emacs_get_tty_pgrp in process.c should be
> changed so that it works on systems that have tcgetpgrp but not
> TIOCGPGRP.  How would you recommend handling this?  Is it necessary to
> use conditional code as in the emacs-23 definition of
> EMACS_GET_TTY_PGRP, or is it safe to just always use tcgetpgrp as in
> the trunk's version of EMACS_GET_TTY_PGRP?

It think your question compounds two issues.

EMACS_GET/SET_TTY_PGRP should die, they are not useful as abstractions anymore.

The comment in emacs_get_tty_pgrp looks scary, it does not look
obvious that the code in the function can be replaced with tcgetpgrp.
Maybe it can, but, IMHO, it needs testing on a few platforms.