This is the mail archive of the cygwin-patches mailing list for the Cygwin project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/1] Cygwin: don't allow getpgrp() to fail

On Jul 24 15:04, Ken Brown wrote:
> On 7/23/2019 3:16 PM, Corinna Vinschen wrote:
> > On Jul 23 19:07, Jon Turney wrote:
> >> On 23/07/2019 17:54, Corinna Vinschen wrote:
> >>> Hi Ken,
> >>>
> >>> On Jul 23 16:12, Ken Brown wrote:
> >>>> According to POSIX, "The getpgrp() function shall always be successful
> >>>> and no return value is reserved to indicate an error."  Cygwin's
> >>>> getpgrp() is defined in terms of getpgid(), which is allowed to fail.
> >>>
> >>> But it shouldn't fail for the current process.  Why should pinfo::init
> >>> fail for myself if it begins like this?
> >>>
> >>>     if (myself && n == myself->pid)
> >>>       {
> >>>         procinfo = myself;
> >>>         destroy = 0;
> >>>         return;
> >>>       }
> >>>
> >>> I fear this patch would only cover up the problem still persisting
> >>> under the hood.
> >>
> >> I agree.
> >>
> >> There is presumably a class of programs which require getpgrp() to return
> >> the correct value for correct operation, which cannot be 0 (since that
> >> cannot be a pid).
> > 
> > However, did we ever see this problem outside of GDB?
> I think I've found the problem, as I just reported on the main cygwin list.  And 
> I agree that my patch was misguided.
> But I still think getpgrp() should be changed, perhaps by having it just return 
> myself->pgid as you suggested earlier.  There's no point in having getpgrp() 
> call getpgid(), which does error checking, when POSIX specifically says "no 
> return value [of getpgrp()] is reserved to indicate an error".  POSIX-compatible 
> applications should call getpgid(0) instead of getpgrp() if they want to do 
> error checking.
> I'll send a couple of patches, one for this issue and one for the tcsetpgrp() 
> problem, so that we can discuss it further.
> Ken

I have a very puzzeling result debugging this.  I just outlined this to
Jon on the #cygwin-developers Freenode IRC channel.  Hopefully your
patch to tcsetpgrp clears this up.

I also found a problem in pinfo::this_proc / pinfo_init while debugging
the above.  I'll post the patch to this list since I would very much
like that you and/or Jon take a close look.


Corinna Vinschen
Cygwin Maintainer

Attachment: signature.asc
Description: PGP signature

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]