RE: Revised^2 precompiled header support for cygwin

["Billinghurst, David \(CALCRTS\)" <david dot billinghurst at comalco dot riotinto dot com dot au>]

> From: Earl Chew 
> 
> Billinghurst, David (CALCRTS) wrote:
> > This patch is based on  Earl Chew's 3.4 patch for 
> precompiled header support for cygwin
> > 	http://gcc.gnu.org/ml/gcc-patches/2004-10/msg01211.html
> > Earl:  Let we know if you no longer want your name 
> associated with this?
> 
> David, thanks for following up and integrating my patch into something
> more recent.
> 
> > Mainline has the additional hooks required, so this is much 
> less intrusive.
> 
> Yes, the new scheme has improvements over the older one.
> 
> > One thing I was unsure about.  In 
> host-cygwin.c(cygwin_gt_pch_get_address)
> > I use fdopen to associate a stream with a file descriptor.  Should I
> > close the stream or not?  I don't see any "bad things" if I 
> leave the 
> > stream open.
> 
> This is not nice in the sense that the FILE* is leaked. In hindsight,
> I think it's better if the code simply use the raw fd as is, rather
> than trying to paste a FILE* over the top of it. At this point, this
> code is Cygwin specific, so there is no need to attempt to try to
> use stdio for portability.
> 
> I think the following would be better:
> 
> static void *
> cygwin_gt_pch_get_address (size_t sz, int fd)
> {
>    void *base;
>    off_t p = lseek(fd, 0, SEEK_CUR);
> 
>    if (p == (off_t) -1)
>      fatal_error ("can't get position in PCH file: %m");
> 
>     /* Cygwin requires that the underlying file be at least
>        as large as the requested mapping.  */
>    if ((size_t) p < sz)
>    {
>      if ( lseek (fd, sz-1, SEEK_SET) != 0
>        || write (fd, '\0', 1) == 1 )
>        fatal_error ("can't extend PCH file: %m");
>    }
> 
>    base = mmap (NULL, sz, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> 
>    if (base == MAP_FAILED)
>      base = NULL;
>    else
>      munmap (base, sz);
> 
>    if (lseek (fd, p, SEEK_SET) != 0)
>      fatal_error ("can't set position in PCH file: %m");
> 
>    return base;
> }

I agree.  I had tried something similar but couldn't get it to work.  
Your code was closer to correct than mine, and fortunately we made 
different mistakes. The following seems to work OK.  I will run a 
bootstrap and test cycle overnight.

static void *
cygwin_gt_pch_get_address (size_t sz, int fd)
{
  void *base;
  off_t p = lseek(fd, 0, SEEK_CUR);

  if (p == (off_t) -1)
    fatal_error ("can't get position in PCH file: %m");

   /* Cygwin requires that the underlying file be at least
      as large as the requested mapping.  */
  if ((size_t) p < sz)
  { 
    if ( lseek (fd, sz-1, SEEK_SET) == (off_t) -1 )
      fatal_error ("can't seek past end of PCH file: %m");
    if ( write (fd, "\0", 1) != 1 )
      fatal_error ("can't extend PCH file: %m");
  }

  base = mmap (NULL, sz, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);

  if (base == MAP_FAILED)
    base = NULL;
  else
    munmap (base, sz);

  if (lseek (fd, p, SEEK_SET) == (off_t) -1 )
    fatal_error ("can't set position in PCH file: %m");

  return base;
}


NOTICE
This e-mail and any attachments are private and confidential and may contain privileged information. If you are not an authorised recipient, the copying or distribution of this e-mail and any attachments is prohibited and you must not read, print or act in reliance on this e-mail or attachments.
This notice should not be removed.