Re: doc: use xmlto pdf

[Corinna Vinschen]

ed on input line 1082.

  [1]
  Underfull \hbox (badness 6675) in paragraph at lines 1261--1265
   []\T1/ptm/m/n/10 A his-tor-i-cal look into the first years of Cyg-win 
de-vel-o
  p-ment is Ge-of-frey J. Noer's

  Underfull \hbox (badness 1) in paragraph at lines 1261--1265
   \T1/ptm/m/n/10 can be found at the [] 2nd USENIX Win-dows NT Sym-po-sium 
On-li
  ne Pro-ceed-ings[][]
  [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]
  Underfull \hbox (badness 4927) in paragraph at lines 2579--2581
  []\T1/ptm/m/n/10 New API: cf-mak-eraw, get_avphys_pages, get_nprocs, 
get_nprocs
  _conf, get_phys_pages,
  [12] [13] [14]
  ! Missing \endcsname inserted.
   
 &
  l.3036 .../README?cvsroot=cygwin-apps&rev=2)">
CVS  
 &
  l.3036 .../README?cvsroot=cygwin-apps&rev=2)">
CVS  Fatal error occurred, no output PDF file produced!
  Transcript written on tmp.log.
  make: *** [cygwin-ug-net/cygwin-ug-net.pdf] Error 1



Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: doc: use xmlto pdf

[Corinna Vinschen]

On Jun 25 19:02, Yaakov S wrote:
> On Fri, 2010-06-25 at 23:14 +0200, Corinna Vinschen wrote:
> > The reason that I changed that to docbook2pdf at one point was that
> > creating a PDF from the docs never worked for me before.
> > 
> > And with your patch it also doesn't work for me on two different Linux
> > systems with different xmlto versions (0.0.18 and 0.0.23).  Here's what
> > happens on Fedora 13, the result is practically the same on the older
> > system.  Maybe you know a solution?
> 
> I was able to duplicate this on a linux VM; it appears to be a problem
> with the passivetex backend.  If I force the dblatex backend, then it
> works, but requires xmlto >= 0.0.21[1].  Could you try the attached
> patch instead?

Looks good.  I updated the older system (which happens to be my default
Cygwin build system) to xmlto 0.0.23 and everything works fine.

Please apply your patch.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: add mkostemp

[Corinna Vinschen]

t;C" char *
>  mktemp(char *path)
>  {
> -  return _gettemp(path, NULL, 0, 0) ? path : (char *) NULL;
> +  return _gettemp(path, NULL, 0, 0, 0) ? path : (char *) NULL;
>  }
> 
>  static int
> -_gettemp(char *path, int *doopen, int domkdir, size_t suffixlen)
> +_gettemp(char *path, int *doopen, int domkdir, size_t suffixlen, int flags)
>  {
>char *start, *trv, *suffp;
>char *pad;
> @@ -105,7 +119,7 @@ _gettemp(char *path, int *doopen, int domkdir, size_t 
> suffixlen)
>  {
>if (doopen)
>   {
> -   if ((*doopen = open (path, O_CREAT | O_EXCL | O_RDWR | O_BINARY,
> +   if ((*doopen = open (path, O_CREAT | O_EXCL | O_RDWR | flags,
>  S_IRUSR | S_IWUSR)) >= 0)
>   return 1;
> if (errno != EEXIST)
> diff --git a/winsup/cygwin/posix.sgml b/winsup/cygwin/posix.sgml
> index 6a3bc22..fdd7589 100644
> --- a/winsup/cygwin/posix.sgml
> +++ b/winsup/cygwin/posix.sgml
> @@ -1043,6 +1043,8 @@ also IEEE Std 1003.1-2008 (POSIX.1-2008).
>  lsetxattr
>  memmem
>  mempcpy
> +mkostemp
> +mkostemps
>  pipe2
>  pow10
>  pow10f
> -- 
> 1.7.1
> 

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: add mkostemp

[Corinna Vinschen]

On Jul 19 12:03, Eric Blake wrote:
> On 07/19/2010 10:45 AM, Corinna Vinschen wrote:
> >> Okay to commit, along with a corresponding patch to doc/new-features.sgml
> >> and a cygwin-specific patch to newlib's stdlib.h?
> > 
> > Yep.  Thanks for the patch.  I CCed the newlib list.  The change to
> > newlib's stdlib.h is preapproved (#ifndef __STRICT_ANSI__ just like
> > mkstemp et al).
> 
> For the record, here's the (pre-approved) patches for newlib and cygwin
> documentation that I'm pushing in tandem with the original cygwin patch.
>  It also fixes a couple of minor problems I noticed while in the area -
> cygwin's cat has been binary-only for some time now, and newlib's
> stdlib.h was not robust to a user file that used #define warning prior
> to including the system header.
> 
> > 
> > Btw., would you mind to enhance newlib's libc/stdio/mktemp.c in the same
> > manner (_ELIX_LEVEL >= 4)?
> 
> The mkostemp[s] additions are guarded by the same levels as mkstemps,
> since all three interfaces are equally non-portable.

Thanks very much for the patch.


Corinna

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat

Re: [PATCH] define RTLD_LOCAL

[Corinna Vinschen]

On Aug  8 00:49, Yaakov S wrote:
>   * include/dlfcn.h (RTLD_LOCAL): Define.

Patch applied.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] implement /proc/filesystems

[Corinna Vinschen]

On Aug  9 01:22, Yaakov S wrote:
>   * fhandler_proc.cc: Add /proc/filesystems virtual file.
>   (format_proc_filesystems): New function.
>   * mount.cc (fs_names): Move to global scope. Redefine as array
>   of { "name", block_device? } structs.
>   (fillout_mntent): Use name member of fs_names.
>   * mount.h (fs_names): New prototype.

Thank you!  Patch applied.  I added an entry to the docs as well.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: res_send() doesn't work with osquery enabled

[Corinna Vinschen]

Pierre, would you mind to take a look?

On Aug 26 19:07, pse...@egg6.net wrote:
> Currently res_init() checks for availability of the native windows
> function DnsQuery_A. If the function is found, it's preferred over the
> cygwin implementation and res_query is set up to use it.
> As DnsQuery_A finds the configured name servers itself, the current code
> assumes we can avoid loading the dns server list with GetNetworkParams().
> 
> However, the assumption that everybody would use res_query is wrong. Some
> programs may use res_mkquery() and res_send() or may only read the list of
> servers from _res.nsaddr_list and send/receive the queries/replies
> themselves. res_send() also relies on nsaddr_list.
> 
> The following patch makes get_dns_info() always try to populate
> nsaddr_list if empty. If resolv.conf exists and provides nameservers, they
> will be used as usual. Otherwise, GetNetworkParams() will be called to get
> the servers from the operating system.
> 
> 2010-08-26  Rumen Stoyanov 
> 
>  * libc/minires-os-if.c (get_dns_info): Always populate nsaddr_list
>  if empty, regardless of the availability of os_query.
>  * libc/minires.c (res_nsend): Make sure there is atleast one
>  nameserver in nsaddr_list or return.

Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: res_send() doesn't work with osquery enabled

[Corinna Vinschen]

On Aug 26 13:16, Pierre A. Humblet wrote:
> - Original Message - 
> From: "Corinna Vinschen" 
> To: 
> Sent: Thursday, August 26, 2010 12:38
> 
> 
> | Pierre, would you mind to take a look?
> | 
> | On Aug 26 19:07, pse...@egg6.net wrote:
> | > Currently res_init() checks for availability of the native windows
> | > function DnsQuery_A. If the function is found, it's preferred over the
> | > cygwin implementation and res_query is set up to use it.
> | > As DnsQuery_A finds the configured name servers itself, the current code
> | > assumes we can avoid loading the dns server list with GetNetworkParams().
> | > 
> | > However, the assumption that everybody would use res_query is wrong. Some
> | > programs may use res_mkquery() and res_send() or may only read the list of
> | > servers from _res.nsaddr_list and send/receive the queries/replies
> | > themselves. res_send() also relies on nsaddr_list.
> 
> It's true that the behavior described above is legitimate, even if nobody had 
> ever 
> requested it. If people want to access nsaddr_list after calling res_ninit, 
> loading 
> iphlpapi.dll every time (as the patch does) is unavoidable.
> 
> The other change has res_nsend return an error if no server can be found.
> Alternatively the error could be reported by res_ninit, by removing the second
> condition in 
> if (statp->nscount == 0 && !statp->os_query) {
> errno = ENONET;
> statp->res_h_errno = NETDB_INTERNAL;
> 
> Hypothetically this could affect some installations where iphlpapi doesn't 
> report any
> servers although the Windows resolver can find a server (but I don't see how 
> this
> could happen), so it's safer to proceed as in the patch.
> However the patch should send errno to ENONET and set res_h_errno to
> NETDB_INTERNAL
> 
> Except for the previous comment, I am fine with the patch.

IIRC you have checkin rights, Pierre.  Please apply whatever you
think is right.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

oes not tend to be changed every
> millisecond! Therefore no reason for every filesystem syscall to
> call it. Caching this further increased the performance.

Does your FS caching take volume mount points into account?

> - File security check: GetSecurityInfo() is implemented in Windows
> in usermode dll (Advapi32.dll). It calls at the end
> NtQuerySecurityObject(). So I implemented a faster version:
> zGetSecurityInfo() which does the same... just faster.

Does your code preserve the inheritance entries which are available via
GetSecurityInfo?  Note that I don't care at all for the GetSecurityInfo
API from a usage POV.  I would prefer to use NtQuerySecurityObject
directly.  However, the sole reason for using GetSecurityInfo is the
fact that NtQuerySecurityObject only returns the plain ACL as it's
stored for the file.  It does not return the ACEs which are inherited
from the parent objects.  These are only available via GetSecurityInfo,
or by checking the parent ACLs manually.

> - symbolic link files: Opening a file and reading its contents is an
> expensive operation. All file cygwin operations must check whether
> the file is a symbolic link or not, which is done by opening the
> file and reading its contents and checking whether it has the
> symlink magic at the beginning of this. Since symbolic link must be

That's not correct.  It only opens files which have cetain DOS flags
set.  Symlinks are either files with a SYSTEM DOS flag, or .lnk files
with the R/O DOS flag set, or reparse points.  So your extension for
the test might help a tiny little bit, but since the SYSTEM attribute
is only rarely set on file which are not Cygwin symlinks, it's a very
rare operation.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Mounting /tmp at TMP or TEMP as a last resort

[Corinna Vinschen]

On Sep  8 15:59, Earl Chew wrote:
> On 08/09/2010 3:41 PM, Christopher Faylor wrote:
> > Thanks for the patch but I don't think this is generally useful.  If you
> > need to mount /tmp somewhere else then it should be fairly trivial to
> > automatically update /etc/fstab.  Corinna may disagree, but I think we
> > should keep the parsing of /etc/fstab as lean as possible; particularly
> > when there are alternatives to modifying Cygwin to achieve the desired
> > result.
> 
> Yes, I understand what you're saying.
> 
> In our situation, we prefer an out-of-the-box deployment. (We're
> essentially trying to get to a "untar this and use it" state).
> 
> That said, I don't think it's possible for /etc/fstab to inspect
> environment variables, so manipulation of /etc/fstab would
> require the assistance of some other script to edit /etc/fstab on
> the fly --- and even then it would be difficult to track changes
> to the environment variables.

Apart from changing /etc/fstab or /etc/fstab.d/$USER by some installer
script, why not just add a one-liner profile script along the lines of

 /etc/profile.d/tmp-mnt.sh:

   mount -f `cygpath -m "${TEMP}"` /tmp


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep  6 15:24, Corinna Vinschen wrote:
> > - GetVolumeInfo: The C:\ drive does not tend to be changed every
> > millisecond! Therefore no reason for every filesystem syscall to
> > call it. Caching this further increased the performance.
> 
> Does your FS caching take volume mount points into account?

We had some filesystem caching back in Cygwin 1.5 already, but the new
code in symlink_info::check and fs_info::update was supposed to make it
unnecessary.

However, I created a new caching mechanism now, which requires only a
single NtQueryVolumeInformationFile call and then uses the result for a
hash.  I also tested a method which uses the object name of the
underlying NT device as key for the hash, but it's actually ~10% slower
than using the content of the FILE_FS_VOLUME_INFORMATION structure
returned by NtQueryVolumeInformationFile(..., FileFsVolumeInformation).
That's a rather unexpected result, but that's Windows.

> > - File security check: GetSecurityInfo() is implemented in Windows
> > in usermode dll (Advapi32.dll). It calls at the end
> > NtQuerySecurityObject(). So I implemented a faster version:
> > zGetSecurityInfo() which does the same... just faster.
> 
> Does your code preserve the inheritance entries which are available via
> GetSecurityInfo?  Note that I don't care at all for the GetSecurityInfo
> API from a usage POV.  I would prefer to use NtQuerySecurityObject
> directly.  However, the sole reason for using GetSecurityInfo is the
> fact that NtQuerySecurityObject only returns the plain ACL as it's
> stored for the file.  It does not return the ACEs which are inherited
> from the parent objects.  These are only available via GetSecurityInfo,
> or by checking the parent ACLs manually.

This turned out to be total nonsense.  The actual reason to use
GetSecurityInfo was the fact that it's the only function which sets the
INHERITED_ACE flag, as my comment in get_file_sd tells everybody who's
willing to read.  So I read that now, too.

A bit of looking through the code shows that the INHERITED_ACE flag is
*only* required if a file has just been created (see alloc_sd).  So only
the call to set_file_attribute in fhandler_base::open() really needs
these flags.

What I did now was to extend get_file_sd to call GetSecurityInfo only in
this case, while every other call to get_file_sd will actually just call
NtQuerySecurityObject, which is obviously faster because it doesn't have
to check the parent ACL.  Additionally I changed the allocation for the
security_descriptor structure so that it's allocated big enough and only
a single call to NtQuerySecurityObject is performed, rather than two.

Other than that I  have another patch in the loop which stores file
information from symlink_info::check to reuse them in calls to stat,
to drop another OS call.  This needs a bit more testing, though.

All in all the number of OS calls accessing "foo" in a single `ls -l
foo' call has dropped from 18 to 12.  `stat foo' needed 10  before,
now it needs 8 or 7, depending on the fs caching state.  `ls -lR'
on a Samba drive now is almost 30% faster.

What I'm still mulling over is a good idea to re-use the results of a
former call to readdir in a stat call.  One problem here is to make sure
that a subsequent stat call is *really* accessing the same file as the
former readdir returned.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep 10 13:23, Christopher Faylor wrote:
> On Fri, Sep 10, 2010 at 05:08:40PM +0200, Corinna Vinschen wrote:
> >What I'm still mulling over is a good idea to re-use the results of a
> >former call to readdir in a stat call.  One problem here is to make sure
> >that a subsequent stat call is *really* accessing the same file as the
> >former readdir returned.
> 
> I've considered that before but you really can't cheaply determine that
> the file hasn't changed without going to the OS.  And, then it isn't cheap.

Yes, that's what it always comes down to.  That's why I'm considering to
restrict this speedup to fstatat.  I wrote more about this on the
cygwin-developers list.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Add fenv.h and support.

[Corinna Vinschen]

Hi Dave,

On Sep 11 08:11, Dave Korn wrote:
> On 11/09/2010 06:10, Christopher Faylor wrote:
> > On Sat, Sep 11, 2010 at 01:42:49AM +0100, Dave Korn wrote:
> >> On 10/09/2010 22:43, Christopher Faylor wrote:
> >>
> >>> Looks nice to me with one HUGE caveat:  Please maintain the pseudo-sorted
> >>> order in cygwin.din.  Sorry to have to impose this burden on you.
> >>  No, that's fine; I've never been sure whether we need to care about the
> >> ordinal numbers or not in that file.  (AFAIK, we don't have any realistic
> >> scenarios where anyone would be linking against the Cygwin DLL by ordinal
> >> imports, but I hate making assumptions based only on my own limited 
> >> experience...)
> > 
> > It never even occurred to me about ordinal numbers but since I've been
> > reorganizing that file for years I guess it hasn't been a problem.
> 
>   I checked.  Something somewhere sorts all the exports it turns out, so they
> all get ordinals assigned in alphanumeric sort order anyway, regardless of
> cygwin.din order.  So, I ended up committing it like so:

Can you please add some words to doc/new-features.sgml?


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Add fenv.h and support.

[Corinna Vinschen]

On Sep 11 12:22, Dave Korn wrote:
> On 11/09/2010 09:09, Corinna Vinschen wrote:
> > Hi Dave,
> 
>   Morning!
> 
> > On Sep 11 08:11, Dave Korn wrote:
> >> So, I ended up committing it like so:
> > 
> > Can you please add some words to doc/new-features.sgml?
> 
>   How's this look?
> 
> winsup/doc/ChangeLog:
> 
>   * new-features.sgml: Mention fenv support.

I would remove the opengroup link and just keep the gnu C lib one,
but other than that it looks good.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep 12 10:49, Yoni Londner wrote:
> Hi,
> 
> The caching-speed up is trivial:
> We store the the FileFullDirectoryInformation fields, and if any of
> them change - we re-read the file.
> 
> Its not (in practical life) possible to change a file without
> causing a modification on 
> FileIndex/CreationTime/LastWriteTime/ChangeTime/EndOfFile/AllocationSize/FileAttributes/FileName/EaSize!
> 
> From the MSDN we see the info we can get on a
> FileFullDirectoryInformation request:

We're already using FileBothDirectoryInformation and
FileBothIdDirectoryInformation in readdir anyway.

However, isn't that kind of a chicken/egg situation?  If you want to
reuse the content of the FILE_BOTH{_ID}_DIRECTORY_INFORMATION structure
from a previous call to readdir, you would have to call the
corresponding NtQueryInformationFile call(s) to fetch the information
from the file for comparision purposes.  When you fetched it anyway,
there's no reason anymore to compare them, since you can use what
you just fetched.  Where's the advantage?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep 12 13:41, Corinna Vinschen wrote:
> On Sep 12 10:49, Yoni Londner wrote:
> > Hi,
> > 
> > The caching-speed up is trivial:
> > We store the the FileFullDirectoryInformation fields, and if any of
> > them change - we re-read the file.
> > 
> > Its not (in practical life) possible to change a file without
> > causing a modification on 
> > FileIndex/CreationTime/LastWriteTime/ChangeTime/EndOfFile/AllocationSize/FileAttributes/FileName/EaSize!
> > 
> > From the MSDN we see the info we can get on a
> > FileFullDirectoryInformation request:
> 
> We're already using FileBothDirectoryInformation and
> FileBothIdDirectoryInformation in readdir anyway.
> 
> However, isn't that kind of a chicken/egg situation?  If you want to
> reuse the content of the FILE_BOTH{_ID}_DIRECTORY_INFORMATION structure
> from a previous call to readdir, you would have to call the
> corresponding NtQueryInformationFile call(s) to fetch the information
> from the file for comparision purposes.  When you fetched it anyway,
> there's no reason anymore to compare them, since you can use what
> you just fetched.  Where's the advantage?

The patch I still have in the loop uses the FILE_NETWORK_OPEN_INFORMATION
structure throughout, starting in symlink_info::check.  The content is
then stored in the path_conv structure and reused in
fhandler_disk_file::fstat_by_handle.  If the content of that structure
would be used as fingerprint, then we could perform readdir/stat caching
without requiring fstatat.  Something similar could be used to perform
ACL caching, so that a typical `ls -l foo' would not request the ACL
two or three times, but only once...


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep 13 13:45, Yoni Londner wrote:
> Hi,
> 
> >> Abstract: I prepared a patch that improves Cygwin Filesystem
> >> performance by x4 on Cygwin 1.7.5 (1.7.5 vanilla 530ms -->  1.7.5
> >> patched 120ms). I ported the patch to 1.7.7, did tests, and found
> >> out that 1.7.7 had a very serious 9x (!) performance degradation
> >> from 1.7.5 (1.7.5 vanilla 530ms -->  1.7.7 vanilla 3900ms -->  1.7.7
> >> patched 3500ms), which does makes this patch useless until the
> >> performance degradation is fixed.
> >
> > The problem is, I can't reproduce such a degradation.  If I run sometimg
> > like `time ls -l /bin>  /dev/null', the times are very slightly better
> > with 1.7.7 than with 1.7.5 (without caching effect 1200ms vs. 1500ms,
> > with caching effect 500ms vs. 620ms on average).  Starting with 1.7.6,
> > Cygwin reused handles from symlink_info::check in stat() and other
> > syscalls.  If there is such degradation under some circumstances, I need
> > a reproducible scenario, or at least some strace output which shows at
> > which point this happens.  Apart from actual patches this should be
> > discussed on the cygwin-developer list.
> >
> 
> First of all, even your results of 1200-1500ms (1st time) and
> 500-600ms (2nd time) is still way way way too long. On linux with an
> NTFS mount of C:/cygwin, this took <2ms!
> 
> And even on Win32 CMD.EXE this same operation will take you less
> than 100ms. which is 5x to 10x faster.
> 
> The main reason for the difference: the Windows CMD.EXE does not
> open file handles, which make the NTFS file system to actually go
> and read each file's first 16KB of contents (even though you did not
> ask for it!).

I'm not exactly concerned about Linux being way faster accessing an NTFS
drive.  After all it's the OS itself and comes with it's own NTFS driver
which obviously is streamlined for typical POSIX operations.

And then there's Win32 which can go through a dir much faster as well,
since it doesn't have to care for POSIX compatibility of the result, and
the OS function calls coincidentally match what a cmd "dir" call needs.

If you're looking for a fair comparision, why don't you look for
Interix?  I did, and what I see is pretty much the same thing we do in
Cygwin.  Actually, with the last Cygwin from CVS an ls -lR on a
non-marginal directory tree is already faster than the same operation
under Interix.

That doesn't mean I won't look for more ways to enhance Cygwin's
performance, but it won't be by adding CYGWIN environment switches
or by neglecting correct information in stat.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep 13 13:28, Yoni Londner wrote:
> Hi,
> 
> > However, isn't that kind of a chicken/egg situation?  If you want to
> > reuse the content of the FILE_BOTH{_ID}_DIRECTORY_INFORMATION structure
> > from a previous call to readdir, you would have to call the
> 
> I am not talking about reusing info from a previous readdir.
> 
> Every single file cygwin tries to access, it does it in a loop,
> trying afterwards to check for *.lnk file.
> 
> Using the directory query operations, it is possible to get this
> info faster:
> instead of getting file info for FOO and then for "FOO.lnk",
> Cygwin can query the directory info for "FOO FOO.LNK" (for the file
> requested, plus its possible symlink file).

I don't understand how you think this should work.  The filter expression
given to NtQueryDirectoryFile is either a constant string and has to match
the filename exactly, or it contains wildcards.  This is documented
behaviour: http://msdn.microsoft.com/en-us/library/ff567047%28VS.85%29.aspx
So, "foo" works, "foo*" works, but a list like "foo foo.exe foo.lnk" 
does not.

There's also the problem of handling NFS shares.  However, I just had an
idea how to speed up symlink_info::check without neglecting NFS shares.
This will take some time, though since it turns a lot of code upside
down.  Stay tuned.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

[Fwd: Fw: res_send() doesn't work with osquery enabled]

[Corinna Vinschen]

- Forwarded message from "Pierre A. Humblet" -

> Date: Thu, 16 Sep 2010 12:43:33 -0400
> From: "Pierre A. Humblet" 
> Subject: Fw: res_send() doesn't work with osquery enabled
> To: Corinna Vinschen 
> 
> Corinna,
> 
> this has not made it to the list so far, not sure why.
> 
> I am leaving on a trip and don't have time to investigate,
> so sending it straight to you.
> 
> Pierre
> 
> - Original Message - 
> From: "Pierre A. Humblet"
> To: cygwin-patches
> Sent: Thursday, September 16, 2010 9:52
> Subject: Re: res_send() doesn't work with osquery enabled
> 
> 
> | - Original Message - 
> | From: "Corinna Vinschen"
> | To: cygwin-patches
> | Sent: Friday, September 10, 2010 5:22
> | 
> | 
> || Pierre?  Ping?
> || 
> | 
> | Corinna,
> | 
> | Sorry, an earlier answer was rejected due to inappropriate subject.
> | 
> | After thinking about it, I don't like mixing calls to the Windows resolver 
> | for res_query and contacting DNS servers directly with res_send,
> | as proposed. So I have a patch where res_send also uses the Windows
> | resolver. 
> | 
> | Unfortunately although I can build cygwin1.dll fine, it's broken,
> | something to do with a NULL stdout (this is from /bin/date)
> |   1 thread 2588.0x4c0  fputc (ch=10, file=0x0)
> | at ../../../../../src/newlib/libc/stdio/fputc.c:101
> | 
> | Not sure what to do, I already did make clean for cygwin.
> | 
> | Also minires used to come with a README explaining the effect of
> | an optional /etc/resolv.conf  (e.g. to bypass the Windows resolver).
> | That information is not present currently [ and nobody asks for it :) ]
> | I wonder if we should add it and where to place it. One option is the 
> | User's Guide. Another option is a custom resolv.conf man page. 
> | What do you think?
> | 
> | Pierre

- End forwarded message -

Re: [Fwd: Fw: res_send() doesn't work with osquery enabled]

[Corinna Vinschen]

On Sep 17 09:03, Corinna Vinschen wrote:
> - Forwarded message from "Pierre A. Humblet" -
> > | Sorry, an earlier answer was rejected due to inappropriate subject.

In theory, that should be fixed.

> > | After thinking about it, I don't like mixing calls to the Windows 
> > resolver 
> > | for res_query and contacting DNS servers directly with res_send,
> > | as proposed. So I have a patch where res_send also uses the Windows
> > | resolver. 
> > | 
> > | Unfortunately although I can build cygwin1.dll fine, it's broken,
> > | something to do with a NULL stdout (this is from /bin/date)
> > |   1 thread 2588.0x4c0  fputc (ch=10, file=0x0)
> > | at ../../../../../src/newlib/libc/stdio/fputc.c:101
> > | 
> > | Not sure what to do, I already did make clean for cygwin.

I don't know either.  I have no such problem with CVS HEAD.

> > | Also minires used to come with a README explaining the effect of
> > | an optional /etc/resolv.conf  (e.g. to bypass the Windows resolver).
> > | That information is not present currently [ and nobody asks for it :) ]
> > | I wonder if we should add it and where to place it. One option is the 
> > | User's Guide. Another option is a custom resolv.conf man page. 
> > | What do you think?

The User's Guide would be a good place, IMHO.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

ed out why, and found out that #1 and #2 don't modify the
> access time of the file, whereas #3 does. This already immediately

I just checked this and I can't see that it does.  If it would do
so, shouldn't the access time be different every time I call stat?

  $ stat foo | grep 'Access: [0-9]'
  Access: 2010-09-09 16:27:20.769055700 +0200
  $ stat foo | grep 'Access: [0-9]'
  Access: 2010-09-09 16:27:20.769055700 +0200
  $ stat foo | grep 'Access: [0-9]'
  Access: 2010-09-09 16:27:20.769055700 +0200

I tried it on Windows XP SP3 and Windows 7.

> [...]
> I would suggest using #2 over #1, since its simpler code-wise, and I
> did not see any serious performance difference between the two.

Ok, how is the performance comparison if stat returns the *correct*
hardlink count and the *correct* POSIX permissions?

And how did you get #2 working?  I tried it on XP and W7, but the result
is exactly as I described above.  It just doesn't work to exchange the
filter expression and set RestartScan to TRUE.  I attached a simple
testcase to show what I did.  Here's what happens.  Note that there is
only a file w32-pwd.exe in my homedir, no w32-pwd or w32-pwd.lnk.

  $ gcc -g -o ntquerydir ntquerydir.c -lntdll
  $ ./ntquerydir C:\\cygwin\\home\\corinna w32-pwd
  Filter: w32-pwd
NtQueryDirectoryFile: 0xc00f
  Filter: w32-pwd.exe
NtQueryDirectoryFile: 0x8006
  Filter: w32-pwd.lnk
NtQueryDirectoryFile: 0x8006
  $ ./ntquerydir C:\\cygwin\\home\\corinna w32-pwd.exe
  Filter: w32-pwd.exe
w32-pwd.exe
  Filter: w32-pwd.exe.exe
w32-pwd.exe
  Filter: w32-pwd.exe.lnk
w32-pwd.exe

As you can see, the RestartScan==TRUE does not replace the filter
expression.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat
#define WINVER 0x0601
#include 
#include 
#include 
#include 
#include 
#include 
#include 

int
main (int argc, char **argv)
{
  BYTE buffer[sizeof (FILE_ID_BOTH_DIRECTORY_INFORMATION)
	  + (NAME_MAX + 1) * sizeof (WCHAR)];
  WCHAR dirpath[MAX_PATH];
  WCHAR filterbuf[NAME_MAX + 1];
  UNICODE_STRING dirname;
  UNICODE_STRING filter;
  OBJECT_ATTRIBUTES attr;
  NTSTATUS status;
  HANDLE dir;
  IO_STATUS_BLOCK io;
  int num;
  USHORT filterlen;

  if (argc < 3)
return 0;

  setlocale (LC_ALL, "");
  wcscpy (dirpath, L"\\??\\");
  if (!strncmp (argv[1], "", 2))
{
  wcscat (dirpath, L"UNC");
  ++argv[1];
}
  mbstowcs (dirpath + wcslen (dirpath), argv[1], 256 - wcslen (dirpath));
  RtlInitUnicodeString (&dirname, dirpath);

  mbstowcs (filterbuf, argv[2], 256);
  RtlInitUnicodeString (&filter, filterbuf);
  filterlen = filter.Length / sizeof (WCHAR);

  InitializeObjectAttributes (&attr, &dirname, 0, 0, 0);
  status = NtOpenFile (&dir, FILE_LIST_DIRECTORY | SYNCHRONIZE,
		   &attr, &io, FILE_SHARE_VALID_FLAGS,
		   FILE_SYNCHRONOUS_IO_NONALERT
		   | FILE_OPEN_FOR_BACKUP_INTENT
		   | FILE_DIRECTORY_FILE);
  if (!NT_SUCCESS (status))
{
  printf ("  NtOpenFile: %p\n", status);
  return 1;
}
  for (num = 0; num < 3; ++num)
{
  printf ("Filter: %ls\n", filter.Buffer);
  status = NtQueryDirectoryFile (dir, NULL, NULL, NULL, &io,
 buffer, sizeof buffer,
 FileIdBothDirectoryInformation, TRUE,
 filter.Length ? &filter : NULL, TRUE);
  if (!NT_SUCCESS (status))
	printf ("  NtQueryDirectoryFile: %p\n", status);
  else
  	{
	  FILE_POSITION_INFORMATION fpi;
	  PFILE_ID_BOTH_DIRECTORY_INFORMATION fdi
	= (PFILE_ID_BOTH_DIRECTORY_INFORMATION) buffer;
	  printf ("  %.*ls\n", fdi->FileNameLength / sizeof (WCHAR),
			   fdi->FileName);
	}
  if (num == 0)
	{
	  wcscpy (filter.Buffer + filterlen, L".exe");
	  filter.Length += 4 * sizeof (WCHAR);
	}
  else if (num == 1)
	wcscpy (filter.Buffer + filterlen, L".lnk");
}
  NtClose (dir);
  return 0;
}

Re: Cygwin Filesystem Performance degradation 1.7.5 vs 1.7.7, and methods for improving performance

[Corinna Vinschen]

On Sep 22 11:32, Corinna Vinschen wrote:
> On Sep 22 07:45, Yoni Londner wrote:
> > I checked out why, and found out that #1 and #2 don't modify the
> > access time of the file, whereas #3 does. This already immediately
> 
> I just checked this and I can't see that it does.  If it would do
> so, shouldn't the access time be different every time I call stat?
> 
>   $ stat foo | grep 'Access: [0-9]'
>   Access: 2010-09-09 16:27:20.769055700 +0200
>   $ stat foo | grep 'Access: [0-9]'
>   Access: 2010-09-09 16:27:20.769055700 +0200
>   $ stat foo | grep 'Access: [0-9]'
>   Access: 2010-09-09 16:27:20.769055700 +0200
> 
> I tried it on Windows XP SP3 and Windows 7.

Did you test this on a "noacl" mount, or on a filesystem which doesn't
keep permissions, like FAT?  If so, then I know what happens.  This is
the executable test in fhandler_base::fstat_helper.  It reads the first
two bytes from the file to identify executables by their magic number.
This is especially done to identify shell scripts by their "#!" magic,
so that they are marked as executable in st_mode.  You can switch this
off by specifing the "exec" or "notexec" mount options.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: patch to add C99 complex

[Corinna Vinschen]

On Oct  6 08:01, Marco Atzeri wrote:
> here is the cygwin follow up of the patch 
> sent to newlib mailing list.
> 
> Marco
> 
> +* cygwin.din ( cacos cacosf cacosh cacoshf carg cargf 
> +casin casinf casinh casinhf catan catanf catanh catanhf
> +ccos ccosf ccosh ccoshf cexp cexpf cimag cimagf clog clogf 
> +conj conjf cpow cpowf cproj cprojf creal crealf 
> +csin csinf csinh csinhf csqrt csqrtf 
> +ctan ctanf ctanh ctanhf): Export new complex math functions 
> +

Patch applied.  I also applied the matching patch to the documentation
and bumped the API version.

Thank you!


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] CJK ambiguous width for non-Unicode charsets

[Corinna Vinschen]

On Nov 17 21:46, Andy Koppe wrote:
> Documentation change to go with the newlib patch at
> http://www.cygwin.com/ml/newlib/2010/msg00604.html:
> 
>   * setup2.sgml (setup-locale-ov): Document CJK ambiguous width change
>   for non-Unicode charsets.
>   * new-features.sgml (ov-new1.7.8): Mention CJK ambiguous width change.
> 
> (Btw, "Drop support for Windows NT4 prior to Service Pack 4" appears
> twice in new-features.sgml).

Thanks, applied, including to remove one of the NT4 paragraphs.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix permissions of DEF_CLASS_OBJ ACL entry

[Corinna Vinschen]

On Dec  7 21:53, Christian Franke wrote:
> Cygwin returns permissions 0777 in the DEF_CLASS_OBJ
> ("default:mask:") ACL entry. This patch changes this to 07. The
> upper bits 0770 probably do not make any sense here.
> 
> The value 0777 is one reason why rsync may set bogus permissions.
> (The other reason is that rsync always expects a DEF_OTHER_OBJ
> entry. This is likely a rsync bug which should be fixed upstream)
> See http://cygwin.com/ml/cygwin/2010-11/msg00429.html
> 
> Christian
> 

> 2010-12-07  Christian Franke
> 
>   * sec_acl.cc (getacl): Set DEF_CLASS_OBJ permissions
>   to 07 instead of 0777.

Thanks for catching this.  Applied.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Ensure that the default ACL contains the standard entries

[Corinna Vinschen]

Hi Christian,

On Dec 10 23:05, Christian Franke wrote:
> The ACL from Cygwin always contains the three (USER|GROUP|OTHER)_OBJ
> entries. It might be existing practice elsewhere to return these
> entries also in the default ACL. The attached patch adds these
> entries with empty permissions if necessary.
> 
> The patch would fix this rsync issue:
> http://cygwin.com/ml/cygwin/2010-11/msg00429.html
> 
> The logic for DEF_CLASS_OBJ is unchanged.

Thanks for the patch.  There are two problem with it, unfortunately.
Consider the setfacl tool.  The -m option basically works like this:

  acl (path, GETACL);
  modify_acl ();
  acl (path, SETACL);

Now, what happens with your patch is this.  Let's assume I add a
single default entry:

  $ getfacl dir
  [...]
  user::rwx
  group::r-x
  mask::rwx
  other::r-x
  $ setfacl -m d:u:corinna:rwx dir
  [...]
  user::rwx
  group::r-x
  mask::rwx
  other::r-x
  default:user::---
  default:user:corinna:rwx
  default:group::---
  default:mask::rwx
  default:other::---

This looks good, except that the faked default entries for group and
other are set to 0.  That's rather unexpected.  Actually, by default the
default entries should reflect the standard permission bits.  At least
that's what happens in the above example on Linux (I tried with
different values for the permission bits):

  $ setfacl -m d:u:corinna:rwx dir
  [...]
  user::rwx
  group::r-x
  mask::rwx
  other::r-x
  default:user::rwx
  default:user:corinna:rwx
  default:group::r-x
  default:mask::rwx
  default:other::r-x

This is rather easy to fix (and you added comments in that place), but
here comes problem #2.  In reality, the Windows ACL does not contain any
default entries except for the default entry for user corinna:

  $ icacls dir
  c:\cygwin\home\corinna\dir VINSCHEN\corinna:(F)
 VINSCHEN\vinschen:(RX)
 Everyone:(RX)
 VINSCHEN\corinna:(OI)(CI)(IO)(RX,W,DC)

Ok, but, what happens if I call setfacl again?  The first call to acl
in setfacl returns the faked default entries.  So, after modifying the
acl according to the command line, the SETACL call now still contains
the faked acl entry.  Which means, they are now written back to the
dir's ACL.  Just call setfacl with the same command line again:

  $ setfacl -m d:u:corinna:rwx dir
  $ icacls dir
  c:\cygwin\home\corinna\tmp VINSCHEN\corinna:(F)
 VINSCHEN\vinschen:(RX)
 Everyone:(RX)
 CREATOR OWNER:(OI)(CI)(IO)(D,Rc,WDAC,WO,RA,WA)
 VINSCHEN\corinna:(OI)(CI)(IO)(RX,W,DC)
 CREATOR GROUP:(OI)(CI)(IO)(Rc,RA)
 Everyone:(OI)(CI)(IO)(Rc,RA)

Even though nothing has changed, the ACL is now different since it
actually reflects the so far faked default entries.  I'm not sure if
that's feasible behaviour.  Besides, due to the faked default entries
defaulting to 0 permissions, subsequently created files in that
directory will have 000 permissions by default.  Uh oh.

I'm not entirely sure yet, but maybe the acl function should not
fake these default entries.  From my POV it seems a better approach
if acl(SETACL) actually creates these entries if *any* default entry
is in the incoming acl.  And, it should create these entries with
useful permission values.  This seems to reflect the Linux behaviour
much closer.  What do you think?  Would you implement this?

Btw., while testing your patch I found a bug in setfacl which disallowed
to delete user/group-specific default entries.  I opted for rewriting the
function which examines an incoming acl entry (getaclentry).  Would you
mind to test this bit, too?  The new code accepts a trailing colon, but
I think that's ok.  The SGI setfacl tool used on Linux is even more
relaxed syntax-wise and also accepts trailing colons.


Corinna

Re: [PATCH] Ensure that the default ACL contains the standard entries

[Corinna Vinschen]

On Dec 14 22:22, Christian Franke wrote:
> Hi Corinna,
> 
> Corinna Vinschen wrote:
> >Hi Christian,
> >
> >On Dec 10 23:05, Christian Franke wrote:
> >>The ACL from Cygwin always contains the three (USER|GROUP|OTHER)_OBJ
> >>entries. It might be existing practice elsewhere to return these
> >>entries also in the default ACL. The attached patch adds these
> >>entries with empty permissions if necessary.
> >>
> >>The patch would fix this rsync issue:
> >>http://cygwin.com/ml/cygwin/2010-11/msg00429.html
> >>
> >>The logic for DEF_CLASS_OBJ is unchanged.
> >
> >This looks good, except that the faked default entries for group and
> >other are set to 0.  That's rather unexpected. ...
> >
> >This is rather easy to fix (and you added comments in that place), ...
> 
> New patch attached.

Thanks, applied.

> >I'm not entirely sure yet, but maybe the acl function should not
> >fake these default entries.  From my POV it seems a better approach
> >if acl(SETACL) actually creates these entries if *any* default entry
> >is in the incoming acl.  And, it should create these entries with
> >useful permission values.  This seems to reflect the Linux behaviour
> >much closer.  What do you think?
> 
> AFIAK a minimal ACL must contain the three entries u/g/o which are
> equivalent to the mode permission bits. The default ACL has likely
> the same requirement.

Apparently yes.  I just tested this on Linux and Solaris.  On Linux
the missing entries are added with default values, on Solaris 10
you are required to enter at least the three default o/g/u entries,
otherwise setfacl prints an error message

 "Missing user/group owner, other, mask entry"

> If this is the case, then I would suggest to do both:
> 
> 1. Fake these entries in acl(GETACL) if required. This would ensure
> that the default ACL is complete even if the Windows ACL was not
> created by Cygwin.
> 
> 2. Create these entries in acl(SETACL) if required. This would
> ensure that the following reasonable requirement holds if the
> Windows ACL was created by Cygwin before:
> 
> - "getfacl foo | setfacl -f - foo" should not change the (Windows)
> ACL of "foo".

Ok, fine with me.

> >   Would you implement this?
> 
> Yes, but may take some time.

No worries.  We won't release 1.7.8 before January.

> >Btw., while testing your patch I found a bug in setfacl which disallowed
> >to delete user/group-specific default entries.  I opted for rewriting the
> >function which examines an incoming acl entry (getaclentry).  Would you
> >mind to test this bit, too?  The new code accepts a trailing colon, but
> >I think that's ok.  The SGI setfacl tool used on Linux is even more
> >relaxed syntax-wise and also accepts trailing colons.
> 
> I've done a few test, looks good.

Thank you!

> An unrelated issue found during testing:
> 
> mkdir() may duplicate Windows ACL entries. Testcase (German XP SP3):
> [...]
> Problem in security.cc:alloc_sd() ?

Indeed.  Thanks for the report.  I fixed that in CVS, hopefully.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Ensure that the default ACL contains the standard entries

[Corinna Vinschen]

Hi Christian,

On Dec 15 19:46, Christian Franke wrote:
> Corinna Vinschen wrote:
> >>
> >>New patch attached.
> >Thanks, applied.
> >
> 
> Thanks - rsync issue is now fixed.

Good start.

> >>mkdir() may duplicate Windows ACL entries. Testcase (German XP SP3):
> >>[...]
> >>Problem in security.cc:alloc_sd() ?
> >Indeed.  Thanks for the report.  I fixed that in CVS, hopefully.
> >
> >
> 
> At least the testcase is now OK :-)

Indeed, but the patch was still wrong.  It just droped all ACEs for
creator_owner, creator_group and other in the "unrelated ACE copy loop"
unconditionally.  In fact dropping them is only allowed for newly
created directories.  I applied another patch which should work better.

> BTW: Are there any long term plans to actually implement the acl "mask" ?
> Should be possible by mapping the "mask" restrictions to deny acl
> entries for each named entry:

There are no such plans, but that doesn't mean I wouldn't take patches
which implement this.  In fact I would be *very* happy to get patches
which improve ACL handling, and I'm not finicky in terms of the type
of enhancement.  Various ideas come to mind:

- Fix acl(2) by handling deny ACEs at all.

- Implement the POSIX 1003.1e functions (maybe simply in terms of
  the existing Solaris API).

- Add missing Solaris ACL functions (acl_get, facl_get, acl_set, facl_set,
  acl_fromtext, acl_totext, acl_free, acl_strip, acl_trivial).

- Add Solaris NFSv4 ACLs, which, coincidentally, are almost equivalent
  to Windows ACLs.  This would work nicely for NTFS ACLs, of course.
  See http://docs.sun.com/app/docs/doc/819-2252/acl-5?l=en&a=view

- Last but not least:  Actually handle "mask".

Adding deny entries which correspond to the mask value sounds like an
interesting idea.  Of course they shouldn't be added if they are not
necessary since deny entries and the problems with the so-called
"canonical ACL order" are such a bloody mess.

OTOH, if you don't fake the mask entry, you need a way to stick the mask
into the Windows ACL.  Even twice, the normal mask and the default mask.

This only works if you have a SID which you use for this purpose.

Hmm...

What about redefining the NULL SID?  Right now three bits in the
NULL SID acess mask are used:

  S_ISUID ->  FILE_APPEND_DATA
  S_ISGID ->  FILE_WRITE_DATA
  S_ISVTX ->  FILE_READ_DATA

I don't see that anything speaks against adding other meanings to
the remaining 29 bits.  For instance:

  mask-r  ->  FILE_READ_EA
  mask-w  ->  FILE_WRITE_EA
  mask-x  ->  FILE_EXECUTE
  def-mask-r  ->  FILE_READ_ATTRIBUTES
  def-mask-w  ->  FILE_WRITE_ATTRIBUTES
  def-mask-x  ->  FILE_DELETE_CHILD

If we do this, we can add an actual mask and we can not only use it
in acl(), but also in alloc_sd().

Does that sound useful?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Ensure that the default ACL contains the standard entries

[Corinna Vinschen]

On Dec 20 22:33, Christian Franke wrote:
> Corinna Vinschen wrote:
> >- Add Solaris NFSv4 ACLs, which, coincidentally, are almost equivalent
> >   to Windows ACLs.  This would work nicely for NTFS ACLs, of course.
> >   See http://docs.sun.com/app/docs/doc/819-2252/acl-5?l=en&a=view
> 
> Yes NFSv4 ACLs would make much sense. Coreutils copy-acl.c
> apparently supports these if available (It copies first POSIX ACL
> and then NTFS ACL). This may allow that 'cp -a source dest' keeps
> the NTFS ACL unchanged.

Sounds good.  We should definitly implement this at one point.

> >- Last but not least:  Actually handle "mask".
> >
> >Adding deny entries which correspond to the mask value sounds like an
> >interesting idea.  Of course they shouldn't be added if they are not
> >necessary since deny entries and the problems with the so-called
> >"canonical ACL order" are such a bloody mess.
> >
> 
> Does this mean "deny ACEs must precede non-deny ACEs" or are there
> more requirements?

The order and the way they are evaluated.  In POSIX the evaluation of
the user permissions always preceeds the evaluation of the group and
other permissions.  In a Windows ACL the ACEs are always evaluated in
the order they are given in the ACL.  So, consider a user is in a
specific group for which access is denied.  Since in canonical order the
deny ACEs preceed the allow ACEs, the access will be denied to this
user, even if the user is the owner of the file and there's an allow ACE
in the ACL.  That has a very unfortunate impact on the way you have to
create certain rights, as I describe in
http://cygwin.com/cygwin-ug-net/ntsec.html#ntsec-mapping

> >What about redefining the NULL SID?  Right now three bits in the
> >NULL SID acess mask are used:
> >
> >   S_ISUID ->   FILE_APPEND_DATA
> >   S_ISGID ->   FILE_WRITE_DATA
> >   S_ISVTX ->   FILE_READ_DATA
> >
> >I don't see that anything speaks against adding other meanings to
> >the remaining 29 bits.  For instance:
> >
> >   mask-r  ->   FILE_READ_EA
> >   mask-w  ->   FILE_WRITE_EA
> >   mask-x  ->   FILE_EXECUTE
> >   def-mask-r  ->   FILE_READ_ATTRIBUTES
> >   def-mask-w  ->   FILE_WRITE_ATTRIBUTES
> >   def-mask-x  ->   FILE_DELETE_CHILD
> >
> >If we do this, we can add an actual mask and we can not only use it
> >in acl(), but also in alloc_sd().
> >
> >Does that sound useful?
> >
> 
> Yes.
> 
> Some few draft 0.0001 ideas:
> 
> setacl: If the mask is set and not 'rwx' then add a NTFS deny ACE
> for each input ACE except 'user::' and 'other:'. The permissions
> bits of all deny ACEs are set equivalent to ~mask. Use current
> algorithm to build remaining NTFS non-deny ACE.
> 
> getacl: If the mask is set and not 'rwx' then use the current
> algorithm but ignore all NTFS deny ACEs with permission bits
> equivalent to ~mask.
> 
> chmod: If a mask is set or the current ACL is not minimal then set
> the mask to group permissions and add deny ACEs accordingly.
> Otherwise set the owner group ACE to group permissions.
> 
> 
> With this ACL:
> 
> user::rwx
> group::r-x
> user:foo:rwx
> group:bar:r-x
> mask:rwx
> other:r-x
> 
> a chmod 0740 would result a NTFS ACL equivalent to:
> 
> deny:group::-wx
> deny:user:foo:-wx
> deny:group:bar:-wx
> user::rwx
> group::r-x # effective:r--
> user:foo:rwx # effective:r--
> group:bar:r-x # effective:r--
> mask:r--
> other:---

That won't work for the reason I outlined above.  The canonical ordering
now disallows read access to the user, even though there's an allow ACE
with allows this access to the user.

The problem is, we can either adhere to the canonical order and are
unable to express certain POSIX permissions.  Or, we abandon the
canonical order (as we already do now in alloc_sd in the border cases
like "rw-r-xrw-") and will suffer whining of Windows tools like the
security tab of the file properties dialog in case the ACL is not
canonical.

What you have to do to get POSIX-like permissions even in case of
complex ACLs like the above is to create the ACL in a POSIX-canonical
order.  POSIX-canonical means, the ACLs must be order user->group->other
in the first place.  Only then deny must preceed allow:

  user 1 deny
  user 1 allow
  user 2 deny
  user 2 allow
  [...]
  group 1 deny
  group 1 allow
  group 2 deny
  group 2 allow
  [...]
  other allow
  mask (null ACE)
  defaults

or

  user 1 deny
  user 2 deny
  user 1 allow
  user 2 allow
  [...]
  group 1 deny
  group 2 deny
  group 1 allow
  group 2 allow
  [...]
  other allow
  mask (null ACE)
  defaults

So the above ACL should be, for instance

  deny:user:foo:-wx
  user::rwx
  deny:group::-wx
  deny:group:bar:-wx
  group::r-x # effective:r--
  user:foo:rwx # effective:r--
  group:bar:r-x # effective:r--
  mask:r--
  other:---


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] cygcheck -s should not imply -d

[Corinna Vinschen]

On Jan  5 19:50, Jon TURNEY wrote:
> 
> Currently, for cygcheck -s implies -d.  This seems rather unhelpful.
> 
> I'm afraid I've lost the thread which inspired this, but in it the reporter
> provided cygcheck -svr output as requested, but this did not help diagnose
> what ultimately turned out to be the problem, that a DLL was actually an older
> version (presumably due to replace-in-use problems)
> 
> Attached a patch to modify cygcheck so -s no longer implies -d (although -d
> can still be used).
> 

> 
> 2011-01-05  Jon TURNEY
> 
>   * cygcheck.cc (main): don't imply -d from -s option to cygcheck

Looks good to me.  Applied.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] cygcheck -s should not imply -d

[Corinna Vinschen]

On Jan 10 12:52, Christopher Faylor wrote:
> On Mon, Jan 10, 2011 at 01:51:02PM +0100, Corinna Vinschen wrote:
> >On Jan  5 19:50, Jon TURNEY wrote:
> >> 
> >> Currently, for cygcheck -s implies -d.  This seems rather unhelpful.
> >> 
> >> I'm afraid I've lost the thread which inspired this, but in it the reporter
> >> provided cygcheck -svr output as requested, but this did not help diagnose
> >> what ultimately turned out to be the problem, that a DLL was actually an 
> >> older
> >> version (presumably due to replace-in-use problems)
> >> 
> >> Attached a patch to modify cygcheck so -s no longer implies -d (although -d
> >> can still be used).
> >> 
> >
> >> 
> >> 2011-01-05  Jon TURNEY
> >> 
> >>* cygcheck.cc (main): don't imply -d from -s option to cygcheck
> >
> >Looks good to me.  Applied.
> 
> Sorry that I didn't reply to this.  I wasn't 100% convinced that this
> was a good idea since some of the packages show up as having problems
> when they are ok.  I was wondering if that would end up generating more
> (understandably) confused mailing list traffic but I guess, in the end,
> it probably is better to check the validity of the packages for the
> prescribed error reporting technique.

I wasn't quite sure either, but while running cygcheck with Jon's patch
it started to make more sense.  We can also change the docs to ask for
`cygcheck -svrd' output, but I guess we should just wait and see.

Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Fixes for cfget[io]speed

[Corinna Vinschen]

On Jan 11 03:36, Yaakov (Cygwin/X) wrote:
> I discovered some compliance issues with cfget[io]speed:
> 
> * POSIX requires these to be declared (at least) as functions[1];
> * POSIX requires their argument to be const[1];
> * the macros are not safe for C++ code.
> 
> The following patch fixes these issues, providing that constifying the
> arguments doesn't change the ABI.
> 
> 
> Yaakov
> 
> [1]
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/termios.h.html

> 2011-01-11  Yaakov Selkowitz
> 
>   * termios.cc (cfgetospeed, cfgetispeed): Constify argument per POSIX.
>   * include/sys/termios.h (cfgetospeed, cfgetispeed): Declare functions.
>   Move macros after declarations and make conditional on !__cplusplus.

Thanks, applied.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] cygcheck -s should not imply -d

[Corinna Vinschen]

On Jan 11 14:26, Jon TURNEY wrote:
> On 11/01/2011 08:10, Corinna Vinschen wrote:
> > I wasn't quite sure either, but while running cygcheck with Jon's patch
> > it started to make more sense.  We can also change the docs to ask for
> > `cygcheck -svrd' output, but I guess we should just wait and see.
> 
> FWIW (I don't have all packages installed), mutt is the only package I have
> installed for which cygcheck -c falsely reports a problem.
> 
> $ cygcheck -c | grep -v OK
> Cygwin Package Information
> PackageVersion  Status
> mutt   1.5.20-1 Incomplete

Do you happen to know why?

> Would a patch to http://cygwin.com/setup.html be welcome recommending that:
> (a) if a package installs files which a user is expected to customize, don't
> trample over those customizations when the package is upgraded/reinstalled

Isn't that what /etc/defaults and /etc/postinstall is for, basically?
I'm not sure I understand what you're proposing.  At which point should
setup warn and how is it supposed to know that a file is a
user-customizable one?  In theory, that's all in the responsibility
of the package.

> (b) a package should verify as correctly installed with cygcheck -c?

I don't understand this, sorry.  Would you mind to rephrase and maybe
give an example what you mean?


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] cygcheck -s should not imply -d

[Corinna Vinschen]

On Jan 13 13:04, Jon TURNEY wrote:
> On 13/01/2011 12:33, Corinna Vinschen wrote:
> > On Jan 11 14:26, Jon TURNEY wrote:
> >> On 11/01/2011 08:10, Corinna Vinschen wrote:
> >>> I wasn't quite sure either, but while running cygcheck with Jon's patch
> >>> it started to make more sense.  We can also change the docs to ask for
> >>> `cygcheck -svrd' output, but I guess we should just wait and see.
> >>
> >> FWIW (I don't have all packages installed), mutt is the only package I have
> >> installed for which cygcheck -c falsely reports a problem.
> >>
> >> $ cygcheck -c | grep -v OK
> >> Cygwin Package Information
> >> PackageVersion  Status
> >> mutt   1.5.20-1 Incomplete
> > 
> > Do you happen to know why?
> 
> You can read my ill-informed speculation about this matter at [1] :-)
> 
> [1] http://sourceware.org/ml/cygwin-apps/2010-11/msg00065.html

Uh, ok.  Thanks for the pointer.

> >> Would a patch to http://cygwin.com/setup.html be welcome recommending that:
> >> (a) if a package installs files which a user is expected to customize, 
> >> don't
> >> trample over those customizations when the package is upgraded/reinstalled
> > 
> > Isn't that what /etc/defaults and /etc/postinstall is for, basically?
> > I'm not sure I understand what you're proposing.  At which point should
> > setup warn and how is it supposed to know that a file is a
> > user-customizable one?  In theory, that's all in the responsibility
> > of the package.
> 
> Sorry, that URL isn't very helpfully named.  I'm not proposing to change
> setup.exe, I'm just suggesting adding some text to the 'Cygwin Package
> Contributor's Guide' web page, recommending those things. (I only became aware
> of the existence of /etc/defaults by looking at what other packages do, I
> can't see it mentioned on that page)

Ouch.  Sorry about that.  Yes, sure, it would surely be welcome
to see progress in the docs, too.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix bogus fsync() error

[Corinna Vinschen]

On Jan 31 20:44, Christian Franke wrote:
> If used on raw devices like /dev/sda fsync() always fails with
> EBADRQC (54) because FlushFileBuffers() always fails with
> ERROR_INVALID_FUNCTION (1).
> 
> The attached patch fixes this by simply ignoring this error in the
> fhandler_base implementation. This should not affect any real flush
> errors which likely would return other error codes.
> 
> An alternative approach would be to ignore the error only in a new
> fhandler_raw_dev/floppy::fsync(). IMO not worth the effort is this
> case.

I agree.  I applied the patch.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix bogus fsync() error

[Corinna Vinschen]

On Feb  1 10:48, Christopher Faylor wrote:
> On Tue, Feb 01, 2011 at 09:47:15AM +0100, Corinna Vinschen wrote:
> >On Jan 31 20:44, Christian Franke wrote:
> >> If used on raw devices like /dev/sda fsync() always fails with
> >> EBADRQC (54) because FlushFileBuffers() always fails with
> >> ERROR_INVALID_FUNCTION (1).
> >> 
> >> The attached patch fixes this by simply ignoring this error in the
> >> fhandler_base implementation. This should not affect any real flush
> >> errors which likely would return other error codes.
> >> 
> >> An alternative approach would be to ignore the error only in a new
> >> fhandler_raw_dev/floppy::fsync(). IMO not worth the effort is this
> >> case.
> >
> >I agree.  I applied the patch.
> 
> Shouldn't this patch be a little more robust and check that you're getting
> ERROR_INVALID_FUNCTION only for raw devices?

Filesystems should never return this error code, afaik.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: provide __xpg_strerror_r

[Corinna Vinschen]

On Feb  5 13:43, Eric Blake wrote:
> On 02/05/2011 01:28 PM, Christopher Faylor wrote:
> > On Sat, Feb 05, 2011 at 01:04:16PM -0700, Eric Blake wrote:
> >> Our strerror_r is lousy (it doesn't even match glibc's behavior); see my
> >> request to the newlib list.
> > 
> > We really should just implement strerror_r in errno.cc.  It doesn't make
> > sense to have two different implementations
> 
> You mean, implement the POSIX interface for strerror_r in errno.cc, and
> ditch glibc compatibility?  But, backwards compatibility demands that we
> have two interfaces - the glibc one that returns char* for satisfying
> the link demands of existing applications, and the POSIX one that
> returns int, so we really are stuck with providing two forms of
> strerror_r if we intend to comply with POSIX.
> 
> We already provide our own strerror() (it provides a better experience
> for out-of-range values that the newlib interface), but we're currently
> using the newlib strerror_r() (in spite of its truncation flaw).
> 
> How should I rework this patch?

It would be better if we implement strerror_r locally, in two versions,
just as on Linux.  I think the best approach is to implement this in
newlib first (I replied to your mail there) and then, given that we use
the newlib string.h, copy the method over to Cygwin to match our current
strerror more closely.

Does that make sense?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Crosscompiling configure fix

[Corinna Vinschen]

On Feb  5 21:34, Peter Foley wrote:
> I've submitted a fix for a problem I came across while trying to build a 
> Linux-hosted Cygwin cross compiler. While bootstrapping Cygwin the autoconf 
> scripts in winsup/cygwin and winsup/cygserver fail because the bootstrap 
> compiler is missing some of the files needed to link a Cygwin executable. 
> Because the source for some of the needed files is in the winsup directory, 
> this creates a curricular dependency. The attached patch lets autoconf 
> complete successfully by not running the tests that require linking if Cygwin 
> is being crosscompiled.
> 
> Thanks,
> 
> Peter Foley
> 
> winsup/cygserver/ChangeLog:
> 
> 2011-02-5 Peter Foley <...>
> 
>   * configure.in: Skip tests that require linking if cross compiling.
>   * configure: Regenerate.
> 
> winsup/cygwin/ChangeLog:
> 
> 2011-02-5 Peter Foley <...>
> 
>   * configure.in: Skip tests that require linking if cross compiling.
>   * configure: Regenerate.

Thanks for the patch.  Btw., you don't have to provide the generated
files, the configure.in files are sufficient.

I'm just wondering why we need this stuff at all.  I mean, is there
really any good reason to do the AC_ALLOCA test, and why do we have
this AC_TRY_COMPILE test for __builtin_memset?  Both results are not
used anywhere, they are just written to config.h and then forgotten.

So I take it, we could just drop this stuff.

Chris?  What do you say?

Index: cygserver/configure.in
===
RCS file: /cvs/src/src/winsup/cygserver/configure.in,v
retrieving revision 1.4
diff -u -p -r1.4 configure.in
--- cygserver/configure.in  24 May 2006 16:59:02 -  1.4
+++ cygserver/configure.in  7 Feb 2011 11:57:29 -
@@ -44,26 +44,8 @@ AC_CHECK_TOOL(NM, nm, nm)
 AC_CHECK_TOOL(DLLTOOL, dlltool, dlltool)
 AC_CHECK_TOOL(WINDRES, windres, windres)
 
-AC_ALLOCA
 AC_PROG_MAKE_SET
 
-dnl check whether gcc supports __builtin_memset.
-# Test for builtin mem* functions.
-AC_LANG_SAVE
-AC_LANG_CPLUSPLUS
-AC_TRY_COMPILE([
-#include 
-void foo(char *s, int c, size_t n)
-{
-  __builtin_memset(s, c, n);
-}
-], [ ],
-use_builtin_memset=yes, use_builtin_memset=no)
-if test $use_builtin_memset = "yes"; then
-  AC_DEFINE(HAVE_BUILTIN_MEMSET)
-fi
-AC_LANG_RESTORE
-
 AC_ARG_ENABLE(debugging,
 [ --enable-debugging   Build a cygwin DLL which has more consistency 
checking for debugging],
 [case "${enableval}" in
Index: cygwin/configure.in
===
RCS file: /cvs/src/src/winsup/cygwin/configure.in,v
retrieving revision 1.34
diff -u -p -r1.34 configure.in
--- cygwin/configure.in 29 Jan 2011 06:41:28 -  1.34
+++ cygwin/configure.in 7 Feb 2011 11:57:29 -
@@ -48,26 +48,8 @@ AC_CHECK_TOOL(RANLIB, ranlib, ranlib)
 AC_CHECK_TOOL(STRIP, strip, strip)
 AC_CHECK_TOOL(WINDRES, windres, windres)
 
-AC_ALLOCA
 AC_PROG_MAKE_SET
 
-dnl check whether gcc supports __builtin_memset.
-# Test for builtin mem* functions.
-AC_LANG_SAVE
-AC_LANG_CPLUSPLUS
-AC_TRY_COMPILE([
-#include 
-void foo(char *s, int c, size_t n)
-{
-  __builtin_memset(s, c, n);
-}
-], [ ],
-use_builtin_memset=yes, use_builtin_memset=no)
-if test $use_builtin_memset = "yes"; then
-  AC_DEFINE(HAVE_BUILTIN_MEMSET)
-fi
-AC_LANG_RESTORE
-
 AC_ARG_ENABLE(debugging,
 [ --enable-debugging   Build a cygwin DLL which has more consistency 
checking for debugging],
 [case "${enableval}" in

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Crosscompiling configure fix

[Corinna Vinschen]

On Feb  7 10:26, Christopher Faylor wrote:
> On Mon, Feb 07, 2011 at 12:58:57PM +0100, Corinna Vinschen wrote:
> >I'm just wondering why we need this stuff at all.  I mean, is there
> >really any good reason to do the AC_ALLOCA test, and why do we have
> >this AC_TRY_COMPILE test for __builtin_memset?  Both results are not
> >used anywhere, they are just written to config.h and then forgotten.
> >
> >So I take it, we could just drop this stuff.
> >
> >Chris?  What do you say?
> 
> I agree.  Nuke 'em.

Done.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: provide __xpg_strerror_r

[Corinna Vinschen]

On Feb  9 21:15, Christopher Faylor wrote:
> On Wed, Feb 09, 2011 at 05:20:59PM -0700, Eric Blake wrote:
> >+/* Newlib's  provides declarations for two strerror_r
> >+   variants, according to preprocessor feature macros.  It does the
> >+   right thing for GNU strerror_r, but its __xpg_strerror_r mishandles
> >+   a case of EINVAL when coupled with our strerror() override.*/
> > #if 0
> 
> Can't we get rid of this now?

I agree.  We should simply implement strerror_r by ourselves, even if
it's identical to newlib's strerror_r.  In the long run it's less
puzzeling to have all the strerror variants in one place.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] pthread_yield

[Corinna Vinschen]

On Feb 10 01:04, Christopher Faylor wrote:
> On Wed, Feb 09, 2011 at 11:49:58PM -0600, Yaakov (Cygwin/X) wrote:
> >pthread_yield(3) was part of the POSIX.1c drafts but never made it into
> >the final standard.  Nevertheless, it is provided by Linux[1],
> >FreeBSD[2], OpenBSD[3], AIX[4], and possibly other *NIXes.  
> >
> >"On Linux, this function is implemented as a call to sched_yield(2)."
> >Patch attached.
> 
> Please check in.

Two notes:

- We should use the API version bump to 236 for both new functions,
  __xpg_strerror_r as well as pthread_yield.

- Please add the new entry point to doc/new-features.sgml.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: provide __xpg_strerror_r

[Corinna Vinschen]

On Feb 10 10:50, Corinna Vinschen wrote:
> On Feb  9 21:15, Christopher Faylor wrote:
> > On Wed, Feb 09, 2011 at 05:20:59PM -0700, Eric Blake wrote:
> > >+/* Newlib's  provides declarations for two strerror_r
> > >+   variants, according to preprocessor feature macros.  It does the
> > >+   right thing for GNU strerror_r, but its __xpg_strerror_r mishandles
> > >+   a case of EINVAL when coupled with our strerror() override.*/
> > > #if 0
> > 
> > Can't we get rid of this now?
> 
> I agree.  We should simply implement strerror_r by ourselves, even if
> it's identical to newlib's strerror_r.  In the long run it's less
> puzzeling to have all the strerror variants in one place.

Oh, and: http://cygwin.com/ml/cygwin-patches/2011-q1/msg00031.html


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] for SIGSEGV, compilation error in gcc 4.6

[Corinna Vinschen]

On Feb 10 05:43, jojelino wrote:
> 2011-02-10

Did you read http://cygwin.com/contrib.html and the "Before you get
started" section?  Did you already send a copyright assignment?

Also, it would be nice if you would add more words to explain what your
patch is doing.  Just a patch with no explanation is not very inviting
to take a look at it at all.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] for SIGSEGV, compilation error in gcc 4.6

[Corinna Vinschen]

On Feb 10 08:42, Charles Wilson wrote:
> On 2/10/2011 7:47 AM, jojelino wrote:
> > On 2011-02-10 19:02, Corinna Vinschen wrote:
> > 
> >> Also, it would be nice if you would add more words to explain what your
> >> patch is doing.  Just a patch with no explanation is not very inviting
> >> to take a look at it at all.
> > 
> > this patch deals with only "two" problem. and this is "first" one.
> > 
> > static char * (*findenv_func)(const char *, int *) = (char *
> > (*)(const char *, int *)) getearly;
> > findenv_func is declared without __stdcall convention, and it is casting
> > getearly having __stdcall convention with function type without
> > __stdcall convention. to fix this problem, add __stdcall to findenv_func.
> 
> "two-liner"
> 
> +typedef char* (__stdcall *pfnenv)(const char*,int*);
> ...
> -static char * (*findenv_func)(const char *, int *) = (char *
> (*)(const char *, int *)) getearly;
> +static pfnenv findenv_func = &getearly;
> 
> 
> > and this is "another" one.
> > 
> > this one deals with compilation error that gcc 4.6 complained. so i just
> > copy & paste __attribute__((regparm (x))) from function declaration to
> > function definition, so i must admit that this one was derived from
> > original cygwin source code. that is, you can fix it without this patch.
> 
> "mechanical repetition of one-liner"
> 
> I think this patch qualifies under the minor change rule for not needing
> an assignment...although it would be good for the OP to go ahead and
> complete the paperwork for future contributions.
> 
> IANAL, blah blah...

Well, if you put it that way...


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] for SIGSEGV, compilation error in gcc 4.6

[Corinna Vinschen]

On Feb 10 21:47, jojelino wrote:
> On 2011-02-10 19:02, Corinna Vinschen wrote:
> 
> >Also, it would be nice if you would add more words to explain what your
> >patch is doing.  Just a patch with no explanation is not very inviting
> >to take a look at it at all.
> 
> this patch deals with only "two" problem. and this is "first" one.
> 
> static char * (*findenv_func)(const char *, int *) = (char *
> (*)(const char *, int *)) getearly;
> findenv_func is declared without __stdcall convention, and it is
> casting getearly having __stdcall convention with function type
> without __stdcall convention. to fix this problem, add __stdcall to
> findenv_func.
> 
> and this is "another" one.
> 
> this one deals with compilation error that gcc 4.6 complained. so i
> just copy & paste __attribute__((regparm (x))) from function
> declaration to function definition, so i must admit that this one
> was derived from original cygwin source code. that is, you can fix
> it without this patch.

Ok, I have just a problem.  Your patch doesn't apply because your
mail client appears to insert line breaks if the lines get too long.
Please send the patch again without the line breaks.  Maybe you could
just attach it to your mail rather than inlining it.

> > Did you read http://cygwin.com/contrib.html and the "Before you get
> > started" section?  Did you already send a copyright assignment?
> 
> what i understood is, i fill out the assignment form and snail it to
> provided address in http://cygwin.com/assign.txt
> 
> i didn't snail it yet.

Would be nice, though.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] for SIGSEGV, compilation error in gcc 4.6

[Corinna Vinschen]

On Feb 10 15:15, Corinna Vinschen wrote:
> On Feb 10 21:47, jojelino wrote:
> > On 2011-02-10 19:02, Corinna Vinschen wrote:
> > 
> > >Also, it would be nice if you would add more words to explain what your
> > >patch is doing.  Just a patch with no explanation is not very inviting
> > >to take a look at it at all.
> > 
> > this patch deals with only "two" problem. and this is "first" one.
> > 
> > static char * (*findenv_func)(const char *, int *) = (char *
> > (*)(const char *, int *)) getearly;
> > findenv_func is declared without __stdcall convention, and it is
> > casting getearly having __stdcall convention with function type
> > without __stdcall convention. to fix this problem, add __stdcall to
> > findenv_func.
> > 
> > and this is "another" one.
> > 
> > this one deals with compilation error that gcc 4.6 complained. so i
> > just copy & paste __attribute__((regparm (x))) from function
> > declaration to function definition, so i must admit that this one
> > was derived from original cygwin source code. that is, you can fix
> > it without this patch.
> 
> Ok, I have just a problem.  Your patch doesn't apply because your
> mail client appears to insert line breaks if the lines get too long.
> Please send the patch again without the line breaks.  Maybe you could
> just attach it to your mail rather than inlining it.

Oh, and, would you mind to create a new patch which is against current
CVS?  It looks like some of your changes collide with changes already
checked in.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] for SIGSEGV, compilation error in gcc 4.6

[Corinna Vinschen]

On Feb 10 23:56, jojelino wrote:
> i am sorry for extra line feed. corrected.
> requesting review.

Thanks, will do.  Would you mind to give us your real name for the
ChangeLog entry?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] for SIGSEGV, compilation error in gcc 4.6

[Corinna Vinschen]

On Feb 10 10:29, Christopher Faylor wrote:
> On Thu, Feb 10, 2011 at 03:15:15PM +0100, Corinna Vinschen wrote:
> >Ok, I have just a problem.  Your patch doesn't apply because your
> >mail client appears to insert line breaks if the lines get too long.
> >Please send the patch again without the line breaks.  Maybe you could
> >just attach it to your mail rather than inlining it.
> 
> Please don't just apply it.  Some of the changes suffered from a cut/paste
> mentality, where the right solution was not always to just add a __stdcall.
> 
> The patch needs to actually be studied and probably applied piecemeal.

Ok, no worries.  If you're looking into that anyway I just drop off from
this thread.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: src/winsup/doc ChangeLog new-features.sgml

[Corinna Vinschen]

On Feb 15 08:47, Eric Blake wrote:
> On 02/15/2011 08:32 AM, corinna wrote:
> > * new-features.sgml (ov-new1.7.8): Document /proc/sys.
> > 
> > Patches:
> > http://sourceware.org/cgi-bin/cvsweb.cgi/src/winsup/doc/ChangeLog.diff?cvsroot=src&r1=1.328&r2=1.329
> > http://sourceware.org/cgi-bin/cvsweb.cgi/src/winsup/doc/new-features.sgml.diff?cvsroot=src&r1=1.64&r2=1.65
> 
> >  File system access via block devices works.  For instance
> > +(note the trailing backslash!)
> > +
> > +bash$ cd /proc/sys/Device/HarddiskVolumeShadowCopy1/
> 
> That's a trailing slash, not backslash.

Fixed.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: FW: [PATCH] Crosscompiling configure fix

[Corinna Vinschen]

On Feb 11 21:37, Peter Foley wrote:
> I've submitted a fix for a problem I came across while trying to build a 
> Linux-hosted Cygwin cross compiler. Autoconf fails in the cygwin and 
> cygserver directories because the bootstrap compiler cannot link. This patch 
> works around this by defining GCC_NO_EXECUTABLES, which causes autoconf to 
> skip tests that involve linking.
> 
> Note: I submitted a previous patch that included this change, however only 
> part of that patch was applied (the removal of AC_ALLOCA) so I am 
> resubmitting the GCC_NO_EXECUTABLES part of the patch.

Why is that necessary?  There are no tests left which try to build
executables.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Ensure that the default ACL contains the standard entries

[Corinna Vinschen]

Hi Christian,

On Dec 16 12:10, Corinna Vinschen wrote:
> On Dec 15 19:46, Christian Franke wrote:
> > BTW: Are there any long term plans to actually implement the acl "mask" ?
> > Should be possible by mapping the "mask" restrictions to deny acl
> > entries for each named entry:
> 
> There are no such plans, but that doesn't mean I wouldn't take patches
> which implement this.  In fact I would be *very* happy to get patches
> which improve ACL handling, and I'm not finicky in terms of the type
> of enhancement.  Various ideas come to mind:
> 
> - Fix acl(2) by handling deny ACEs at all.
> 
> - Implement the POSIX 1003.1e functions (maybe simply in terms of
>   the existing Solaris API).
> 
> - Add missing Solaris ACL functions (acl_get, facl_get, acl_set, facl_set,
>   acl_fromtext, acl_totext, acl_free, acl_strip, acl_trivial).
> 
> - Add Solaris NFSv4 ACLs, which, coincidentally, are almost equivalent
>   to Windows ACLs.  This would work nicely for NTFS ACLs, of course.
>   See http://docs.sun.com/app/docs/doc/819-2252/acl-5?l=en&a=view
> 
> - Last but not least:  Actually handle "mask".

did you have any further look into any of these points?


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Add an additional relocation attempt pass to load_after_fork()

[Corinna Vinschen]

Hi Jon,

On Mar 13 15:07, Jon TURNEY wrote:
> 
> Attached is a patch which avoids a fork failure due to remap error in the
> specific circumstances described in my email [1], by adding an additional pass
> to load_after_fork() which forces the DLL to be relocated by VirtualAlloc()ing
> a block of memory at the load address as well.
> 
> Hopefully it can be seen by inspection that this code doesn't change the
> behaviour of the first two passes, and so will only be changing the behaviour
> in what was an fatal error case before.

Thanks for the patch, but afaics you don't have a copyright assignment
on file with Red Hat.  It's unfortunately required for substantial
patches.  Please see http://cygwin.com/contrib.html, especially the
"Before you get started" section.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Add an additional relocation attempt pass to load_after_fork()

[Corinna Vinschen]

On Mar 14 22:02, Jon TURNEY wrote:
> On 13/03/2011 15:21, Corinna Vinschen wrote:
> > Thanks for the patch, but afaics you don't have a copyright assignment
> > on file with Red Hat.  It's unfortunately required for substantial
> > patches.  Please see http://cygwin.com/contrib.html, especially the
> > "Before you get started" section.
> 
> No problem, I have signed and posted an assignment, although I'm not sure I
> consider this patch 'substantial' :-)

Thanks.  I'm looking forward to get it.

I think your patch is a good idea, but apart from the fact that I have
to wait for your copyright assignment, I'm reluctant to add it to 1.7.9.
As you probably have seen in CVS, I'm adding new stuff only to a
post-1.7.9 branch right now.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Add an additional relocation attempt pass to load_after_fork()

[Corinna Vinschen]

On Mar 15 11:04, Christopher Faylor wrote:
> On Tue, Mar 15, 2011 at 08:53:13AM +0100, Corinna Vinschen wrote:
> >On Mar 14 22:02, Jon TURNEY wrote:
> >> On 13/03/2011 15:21, Corinna Vinschen wrote:
> >> > Thanks for the patch, but afaics you don't have a copyright assignment
> >> > on file with Red Hat.  It's unfortunately required for substantial
> >> > patches.  Please see http://cygwin.com/contrib.html, especially the
> >> > "Before you get started" section.
> >> 
> >> No problem, I have signed and posted an assignment, although I'm not sure I
> >> consider this patch 'substantial' :-)
> >
> >Thanks.  I'm looking forward to get it.
> >
> >I think your patch is a good idea, but apart from the fact that I have
> >to wait for your copyright assignment, I'm reluctant to add it to 1.7.9.
> >As you probably have seen in CVS, I'm adding new stuff only to a
> >post-1.7.9 branch right now.
> 
> And, since this is my code, I'd like to have the final approval on whether
> it goes in or not.

Sure.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Export strchrnul (pending newlib patch)

[Corinna Vinschen]

On Mar 27 22:38, Yaakov (Cygwin/X) wrote:
> Here's the Cygwin patch to export strchrnul(3) once accepted in newlib.
> 
> 
> Yaakov
> 

> 2011-03-27  Yaakov Selkowitz
> 
>   * cygwin.din (strchrnul): Export.
>   * include/cygwin/version.h (CYGWIN_VERSION_API_MINOR): Bump.
>   * posix.sgml (std-gnu): Add strchrnul.

Thanks, applied.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Export strchrnul (pending newlib patch)

[Corinna Vinschen]

On Mar 28 10:48, Christopher Faylor wrote:
> On Sun, Mar 27, 2011 at 10:38:16PM -0500, Yaakov (Cygwin/X) wrote:
> >Here's the Cygwin patch to export strchrnul(3) once accepted in newlib.
> 
> Cygwin already has an implementation of this named strechr written in
> assembly language.  Maybe we should be exporting that.

Yeah, we could do that.  We could keep strechr as inline implementation
in string.h and copy it over to strfunc.cc as exportable non-inline
function strchrnul.

Is our strechr really faster than an implementation which uses the
word-wise pattern matching optimization?  At least as far as the
matching case is concerned.  It will likely be faster in the
non-matching case.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix return value and errno set by sem_init(), sem_destroy() and sem_close()

[Corinna Vinschen]

On Mar 28 18:16, Christopher Faylor wrote:
> On Mon, Mar 28, 2011 at 11:14:03PM +0100, Jon TURNEY wrote:
> >
> >While looking into some mysterious failures of sem_init() in python, I was
> >somewhat surprised to find the following comment in python/thread_pthread.h:
> >
> >> /*
> >>  * As of February 2002, Cygwin thread implementations mistakenly report 
> >> error
> >>  * codes in the return value of the sem_ calls (like the pthread_ 
> >> functions).
> >>  * Correct implementations return -1 and put the code in errno. This 
> >> supports
> >>  * either.
> >>  */
> >
> >While this comment refers to sem_wait() and sem_trywait(), which seem to have
> >been fixed since [1], it seems that sem_init(), sem_destroy() and sem_close()
> >are still non-conformant with SUS in that (i) they do not set errno, and (ii)
> >they don't return -1 on failure, instead returning the value which should be
> >set as errno.
> >
> >2011-03-28  Jon TURNEY  
> >
> > * thread.cc (semaphore::init, destroy, close): Standards conformance
> > fix.  On a failure, return -1 and set errno.
> > * thread.h (semaphore::terminate): Save errno since semaphore::close()
> > may now modify it.
> >
> >[1] http://cygwin.com/ml/cygwin/2002-02/msg01379.html
> 
> Looks good.  Please check in ASAP so this will make it into 1.7.9.

Jon, your copyright assignment has been forwarded to my manager who's
going to sign it tomorrow.  So I took the liberty to check in this change
since I'd like to get out 1.7.9 today.  Actually, today *and* ASAP.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Provide sys/xattr.h

[Corinna Vinschen]

On Mar 29 02:43, Yaakov (Cygwin/X) wrote:
> Historically, the *xattr functions were first provided by SGI libattr
> and prototyped in .  Later, glibc added them under
> [1], and (on Linux) libattr still provides the symbols for
> ABI compatibility but they are now just wrappers.
> 
> (FWIW, Darwin also provides these symbols in [2].)
> 
> This can be seen very clearly in GLib's configure[3], where
>  and libc are tested in tandem, followed by 
> and libattr.  Hence, with only attr/xattr.h present, libattr-devel is
> required not only for building GLib, but the -lattr becomes hardcoded in
> the libtool .la files, meaning that libglib2.0-devel would require
> libattr-devel even though GLib requires no symbols from libattr1.
> 
> I see two ways to resolve this:
> 
> 1) Move include/attr/xattr.h to include/sys/xattr.h, and ship libattr's
> attr/xattr.h in libattr-devel, exactly as is done on Linux:
> 
> 2011-03-29  Yaakov Selkowitz 
> 
>   * include/attr/xattr.h: Move from here...
>   * include/sys/xattr.h: ...to here.
> 
> 2) Install a copy of include/attr/xattr.h as , as in the
> attached patch.

What about just creating a file sys/attr.h which includes attr/attr.h?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Provide sys/xattr.h

[Corinna Vinschen]

On Mar 29 09:53, Corinna Vinschen wrote:
> On Mar 29 02:43, Yaakov (Cygwin/X) wrote:
> > Historically, the *xattr functions were first provided by SGI libattr
> > and prototyped in .  Later, glibc added them under
> > [1], and (on Linux) libattr still provides the symbols for
> > ABI compatibility but they are now just wrappers.
> > 
> > (FWIW, Darwin also provides these symbols in [2].)
> > 
> > This can be seen very clearly in GLib's configure[3], where
> >  and libc are tested in tandem, followed by 
> > and libattr.  Hence, with only attr/xattr.h present, libattr-devel is
> > required not only for building GLib, but the -lattr becomes hardcoded in
> > the libtool .la files, meaning that libglib2.0-devel would require
> > libattr-devel even though GLib requires no symbols from libattr1.
> > 
> > I see two ways to resolve this:
> > 
> > 1) Move include/attr/xattr.h to include/sys/xattr.h, and ship libattr's
> > attr/xattr.h in libattr-devel, exactly as is done on Linux:
> > 
> > 2011-03-29  Yaakov Selkowitz 
> > 
> > * include/attr/xattr.h: Move from here...
> > * include/sys/xattr.h: ...to here.
> > 
> > 2) Install a copy of include/attr/xattr.h as , as in the
> > attached patch.
> 
> What about just creating a file sys/attr.h which includes attr/attr.h?

Well, with 'x', of course.  Like this:

=== SNIP ===
/* sys/xattr.h

   Copyright 2011 Red Hat, Inc.

This file is part of Cygwin.

This software is a copyrighted work licensed under the terms of the
Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
details. */

/* sys/xattr.h header file for Cygwin.  */

#ifndef _SYS_XATTR_H
#define _SYS_XATTR_H

#include 

#endif /* _SYS_XATTR_H */
=== SNAP ===


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Provide sys/xattr.h

[Corinna Vinschen]

On Mar 29 03:02, Yaakov (Cygwin/X) wrote:
> On Tue, 2011-03-29 at 09:53 +0200, Corinna Vinschen wrote:
> > On Mar 29 02:43, Yaakov (Cygwin/X) wrote:
> > > I see two ways to resolve this:
> > > 
> > > 1) Move include/attr/xattr.h to include/sys/xattr.h, and ship libattr's
> > > attr/xattr.h in libattr-devel, exactly as is done on Linux:
> > > 
> > > 2) Install a copy of include/attr/xattr.h as , as in the
> > > attached patch.
> > 
> > What about just creating a file sys/attr.h which includes attr/attr.h?
> 
> Right, that should do it as well.  I was so fixed on the Linux situation
> (where you have two practically identical headers from different
> sources) that I couldn't think of anything else.  I think it's time for
> bed.

No worries.  I've checked that in and will create the 1.7.9 release now.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] /proc/loadavg: add running/total processes

[Corinna Vinschen]

On Mar 29 19:32, Yaakov (Cygwin/X) wrote:
> This patch adds the fourth component of Linux's /proc/loadavg[1], the
> current running/total processes count.  My only question is if states
> other than 'O' and 'R' should be considered "running" for this purpose.

That looks right.  But I don't see that get_process_state will ever
generate an 'O'.  Wouldn't that be the difference between StateReady (R)
and StateRunning (O)?

> -  return __small_sprintf (destbuf, "%u.%02u %u.%02u %u.%02u\n",
> - 0, 0, 0, 0, 0, 0);
> +  return __small_sprintf (destbuf, "%u.%02u %u.%02u %u.%02u %u/%u\n",
> + 0, 0, 0, 0, 0, 0, running, pids.npids);

What about the last column in /proc/loadavg, the last pid?  Shouldn't
this be added and set to 0 as well?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] /proc/loadavg: add running/total processes

[Corinna Vinschen]

On Mar 30 03:54, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-03-30 at 10:13 +0200, Corinna Vinschen wrote:
> > On Mar 29 19:32, Yaakov (Cygwin/X) wrote:
> > > This patch adds the fourth component of Linux's /proc/loadavg[1], the
> > > current running/total processes count.  My only question is if states
> > > other than 'O' and 'R' should be considered "running" for this purpose.
> > 
> > That looks right.  But I don't see that get_process_state will ever
> > generate an 'O'.
> 
> Good point.  get_process_state() returns only R/S/Z, but
> format_process_status() has a case for a few other states.  Why?

Dunno.  The code has been contributed by Christopher January in 2002
and is essentially unchanged since then.  I don't think anybody would
be angry with you if you're going to pick it up and dust it off a bit...

> > What about the last column in /proc/loadavg, the last pid?  Shouldn't
> > this be added and set to 0 as well?
> 
> I don't think using a 0 is a good idea, in case some software scanf()s
> this file and tries to do something with the information (division by
> zero comes to mind).
> 
> As for actually implementing the fifth column, I wasn't sure as to the
> true significance of this number: is it really just the pid of the last
> process or the number of processes launched since startup?  On Linux,
> AFAICS these are one and the same, as pids are allocated sequentially,
> but not on Cygwin.  I know the wording implies the former but the
> purpose of this file makes me suspect the latter.  Insight, anyone?

Not me.  I only know what the URL you sent is saying.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Add an additional relocation attempt pass to load_after_fork()

[Corinna Vinschen]

Hi Jon,

On Mar 15 16:46, Corinna Vinschen wrote:
> On Mar 15 11:04, Christopher Faylor wrote:
> > On Tue, Mar 15, 2011 at 08:53:13AM +0100, Corinna Vinschen wrote:
> > >On Mar 14 22:02, Jon TURNEY wrote:
> > >> On 13/03/2011 15:21, Corinna Vinschen wrote:
> > >> > Thanks for the patch, but afaics you don't have a copyright assignment
> > >> > on file with Red Hat.  It's unfortunately required for substantial
> > >> > patches.  Please see http://cygwin.com/contrib.html, especially the
> > >> > "Before you get started" section.
> > >> 
> > >> No problem, I have signed and posted an assignment, although I'm not 
> > >> sure I
> > >> consider this patch 'substantial' :-)
> > >
> > >Thanks.  I'm looking forward to get it.
> > >
> > >I think your patch is a good idea, but apart from the fact that I have
> > >to wait for your copyright assignment, I'm reluctant to add it to 1.7.9.
> > >As you probably have seen in CVS, I'm adding new stuff only to a
> > >post-1.7.9 branch right now.
> > 
> > And, since this is my code, I'd like to have the final approval on whether
> > it goes in or not.
> 
> Sure.

Your copyright assignment has been ountersigned by my manager today.
Chris, are you going to take a look into this patch?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] compile cyglsa with mingw-w64

[Corinna Vinschen]

On Mar 30 23:37, Yaakov (Cygwin/X) wrote:
> The mingw-w64 toolchain can now be used in place of MSVC to build
> cyglsa64.dll.  I didn't integrate this into the Makefile because that
> would make the toolchain a hard build-time requirement, and I don't
> think that is desirable at this time.
> 
> Patch and new file attached.
> 
> 
> Yaakov
> 
> 2011-03-30  Yaakov Selkowitz  <...>
> 
>   * cyglsa.c: Fix compilation with MinGW-w64 toolchains.
>   * make-64bit-version-with-mingw-w64.sh: New file.

Thank you, that's a nice change.  I've just applied it.  I'll go
a step further and drop the "build with Visual C" stuff entirely.
I've also another change in the loop which removes the dependency
to advapi32, which I'll apply later today.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] /proc/loadavg: add running/total processes

[Corinna Vinschen]

On Apr  1 02:21, Yaakov (Cygwin/X) wrote:
> On Wed, 2011-03-30 at 12:27 +0200, Corinna Vinschen wrote:
> > On Mar 30 03:54, Yaakov (Cygwin/X) wrote:
> > > On Wed, 2011-03-30 at 10:13 +0200, Corinna Vinschen wrote:
> > > > On Mar 29 19:32, Yaakov (Cygwin/X) wrote:
> > > > > This patch adds the fourth component of Linux's /proc/loadavg[1], the
> > > > > current running/total processes count.  My only question is if states
> > > > > other than 'O' and 'R' should be considered "running" for this 
> > > > > purpose.
> > > > 
> > > > That looks right.  But I don't see that get_process_state will ever
> > > > generate an 'O'.
> > > 
> > > Good point.  get_process_state() returns only R/S/Z, but
> > > format_process_status() has a case for a few other states.  Why?
> > 
> > Dunno.  The code has been contributed by Christopher January in 2002
> > and is essentially unchanged since then.  I don't think anybody would
> > be angry with you if you're going to pick it up and dust it off a bit...
> 
> That's alright, I think I'll leave that alone for now; I have other
> patches waiting in my queue to deal with.
> 
> Bottom line: should I remove the case 'O' since get_process_state
> doesn't (yet?) return it, or leave it in as in format_process_status()?

Leave it and check in your patch.  Could you please also add a FIXME
comment to get_process_state, somewhere around the lines which generate
the 'R' flag, to explain that we might want to revist this code to
generate an 'O' flag at one point?

> Then I would like to leave the fifth column for a later time.

Ok.  Go ahead.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] implement /proc/sysvipc/*

[Corinna Vinschen]

On Apr  1 04:30, Yaakov (Cygwin/X) wrote:
> These patches implement /proc/sysvipc/*, as found on Linux[1]:
> 
> $ ls -l /proc
> [...]
> dr-xr-xr-x 2 Yaakov None   0 Apr  1 04:12 sysvipc/
> [...]
> 
> $ ls -l /proc/sysvipc
> total 0
> -r--r--r-- 1 Yaakov None 0 Apr  1 04:12 msg
> -r--r--r-- 1 Yaakov None 0 Apr  1 04:12 sem
> -r--r--r-- 1 Yaakov None 0 Apr  1 04:12 shm
> 
> # yes, these lines are very long
> $ cat /proc/sysvipc/shm 
>key  shmid perms   size  cpid  lpid nattch   uid   gid cuid   
> cgid  atime  dtime  ctime
>  0 196608  6600 393216  4960  4996  2  1001   513  1001   
> 513 1301639749  0 1301639749
>  0  65537  6600 393216  4916  4996  2  1001   513  1001   
> 513 1301639750  0 1301639750
> [...]
> 
> If cygserver is not running, then the /proc/sysvipc directory still
> exists but readdir()s as empty, and the files therein are nonexistent:
> 
> $ ls /proc/sysvipc/
> 
> $ ls /proc/sysvipc/shm
> ls: cannot access /proc/sysvipc/sem: No such file or directory
> 
> $ cat /proc/sysvipc/shm
> cat: /proc/sysvipc/shm: No such file or directory
> 
> The code uses some hints from the Cygwin modifications to ipcs(1).
> 
> Patch and new file for winsup/cygwin, and patch for winsup/doc attached.
> 
> 
> Yaakov
> 
> 
> [1] 
> http://docs.redhat.com/docs/en-US/Red_Hat_Enterprise_Linux/4/html/Reference_Guide/s2-proc-dir-sysvipc.html
> 

> 2011-04-01  Yaakov Selkowitz  <...>
> 
>   * new-features.sgml (ov-new1.7.10): Document /proc/sysvipc/.
> 

> 2011-04-01  Yaakov Selkowitz  <...<
> 
>   Implement /proc/sysvipc/*
>   * devices.in (dev_procsysvipc_storage): Add.
>   * devices.cc: Regenerate.
>   * devices.h (fh_devices): Add FH_PROCSYSVIPC.
>   * dtable.cc (build_fh_pc): Add case FH_PROCSYSVIPC.
>   * fhandler.h (class fhandler_procsysvipc): Declare.
>   (fhandler_union): Add __procsysvipc.
>   * fhandler_proc.cc (proc_tab): Add sysvipc virt_directory.
>   * fhandler_procsysvipc.cc: New file.
>   * Makefile.in (DLL_OFILES): Add fhandler_procsysvipc.o.
>   * path.h (isproc_dev): Add FH_PROCSYSVIPC to conditional.

Cool stuff.  Thanks for this patch.  However, your patch shows a
problem:

> Index: path.h
> ===
> RCS file: /cvs/src/src/winsup/cygwin/path.h,v
> retrieving revision 1.154
> diff -u -r1.154 path.h
> --- path.h17 Jan 2011 14:19:39 -  1.154
> +++ path.h20 Feb 2011 08:24:53 -
> @@ -19,7 +19,7 @@
>  
>  #define isproc_dev(devn) \
>(devn == FH_PROC || devn == FH_REGISTRY || devn == FH_PROCESS || \
> -   devn == FH_PROCNET || devn == FH_PROCSYS)
> +   devn == FH_PROCNET || devn == FH_PROCSYS || devn == FH_PROCSYSVIPC)

The definition of isproc_dev starts to get on my nerves.  We have to
check for six distinct values now.  I think we should really change
the definition.  Here's what we have in devices.h right now:

  FH_PROC= FHDEV (0, 250),
  FH_REGISTRY= FHDEV (0, 249),
  FH_PROCESS = FHDEV (0, 248),

  FH_FS  = FHDEV (0, 247),  /* filesystem based device */

  FH_NETDRIVE= FHDEV (0, 246),
  FH_DEV = FHDEV (0, 245),
  FH_PROCNET = FHDEV (0, 244),
  FH_PROCESSFD = FHDEV (0, 243),
  FH_PROCSYS = FHDEV (0, 242),
  FH_PROCSYSVIPC = FHDEV (0, 241),

Chris, do you think there's anything speaking against rearranging this
so that the FH_FS and FH_NETDRIVE definitions are separate from the
stuff under /proc?  Or, hang on, we should change all PROC values,
along these lines:

  FH_FS  = FHDEV (0, 247),  /* filesystem based device */
  FH_NETDRIVE= FHDEV (0, 246),
  FH_DEV = FHDEV (0, 245),

  FH_PROC= FHDEV (0, 244),
  FH_REGISTRY= FHDEV (0, 243),
  FH_PROCESS = FHDEV (0, 242),
  FH_PROCNET = FHDEV (0, 241),
  FH_PROCESSFD = FHDEV (0, 240),
  FH_PROCSYS = FHDEV (0, 239),
  FH_PROCSYSVIPC = FHDEV (0, 238),

  FH_PROC_MIN_MINOR = FHDEV (0, 200),   /* Arbitrary value */

Then we can simplify the isproc_dev definition like this:

#define isproc_dev(devn) \
(devn >= FH_PROC_MIN_MINOR && devn <= FH_PROC)

Does that sound ok?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] implement /proc/sysvipc/*

[Corinna Vinschen]

On Apr  1 11:34, Christopher Faylor wrote:
> On Fri, Apr 01, 2011 at 12:05:56PM +0200, Corinna Vinschen wrote:
> >Chris, do you think there's anything speaking against rearranging this
> >so that the FH_FS and FH_NETDRIVE definitions are separate from the
> >stuff under /proc?  Or, hang on, we should change all PROC values,
> >along these lines:
> >
> >  FH_FS  = FHDEV (0, 247),  /* filesystem based device */
> >  FH_NETDRIVE= FHDEV (0, 246),
> >  FH_DEV = FHDEV (0, 245),
> >
> >  FH_PROC= FHDEV (0, 244),
> >  FH_REGISTRY= FHDEV (0, 243),
> >  FH_PROCESS = FHDEV (0, 242),
> >  FH_PROCNET = FHDEV (0, 241),
> >  FH_PROCESSFD = FHDEV (0, 240),
> >  FH_PROCSYS = FHDEV (0, 239),
> >  FH_PROCSYSVIPC = FHDEV (0, 238),
> >
> >  FH_PROC_MIN_MINOR = FHDEV (0, 200),/* Arbitrary value */
> >
> >Then we can simplify the isproc_dev definition like this:
> >
> >#define isproc_dev(devn) \
> > (devn >= FH_PROC_MIN_MINOR && devn <= FH_PROC)
> >
> >Does that sound ok?
> 
> Yes.  I was, for a while, trying to keep the device numbers the same as
> Linux but I don't think that even applies in this case.

Cool, thanks.

Yaakov, would you mind to apply youir patch with this change?


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] implement /proc/sysvipc/*

[Corinna Vinschen]

On Apr  1 14:57, Yaakov (Cygwin/X) wrote:
> On Fri, 2011-04-01 at 12:05 +0200, Corinna Vinschen wrote:
> > Chris, do you think there's anything speaking against rearranging this
> > so that the FH_FS and FH_NETDRIVE definitions are separate from the
> > stuff under /proc?  Or, hang on, we should change all PROC values,
> > along these lines:
> > 
> >   FH_FS  = FHDEV (0, 247),  /* filesystem based device */
> >   FH_NETDRIVE= FHDEV (0, 246),
> >   FH_DEV = FHDEV (0, 245),
> > 
> >   FH_PROC= FHDEV (0, 244),
> >   FH_REGISTRY= FHDEV (0, 243),
> >   FH_PROCESS = FHDEV (0, 242),
> >   FH_PROCNET = FHDEV (0, 241),
> >   FH_PROCESSFD = FHDEV (0, 240),
> >   FH_PROCSYS = FHDEV (0, 239),
> >   FH_PROCSYSVIPC = FHDEV (0, 238),
> > 
> >   FH_PROC_MIN_MINOR = FHDEV (0, 200),   /* Arbitrary value */
> > 
> > Then we can simplify the isproc_dev definition like this:
> > 
> > #define isproc_dev(devn) \
> > (devn >= FH_PROC_MIN_MINOR && devn <= FH_PROC)
> > 
> > Does that sound ok?
> 
> That would mean that the /proc directories range would be right in the
> middle of the major-device-0 range, with non-/proc stuff before and
> after.  For the sake of clarity, I would reorder it a bit further to
> make FH_PROC and friends to one side of major-0 and everything else to
> the other side:
> 
>   /* begin /proc directories */
>   FH_PROC= FHDEV (0, 255),
>   FH_REGISTRY= FHDEV (0, 254),
>   FH_PROCNET = FHDEV (0, 253),
>   FH_PROCESSFD = FHDEV (0, 252),
>   FH_PROCSYS = FHDEV (0, 251),
>   FH_PROCSYSVIPC = FHDEV (0,250),
> 
>   FH_PROC_MIN_MINOR = FHDEV (0,200),
>   /* end /proc directories */
> 
>   FH_PIPE= FHDEV (0, 199),
>   FH_PIPER   = FHDEV (0, 198),
>   FH_PIPEW   = FHDEV (0, 197),
>   FH_FIFO= FHDEV (0, 196),
>   FH_PROCESS = FHDEV (0, 195),
>   FH_FS  = FHDEV (0, 194),/* filesystem based device */
>   FH_NETDRIVE= FHDEV (0, 193),
>   FH_DEV = FHDEV (0, 192),
> 
> As either way this should be a separate changeset IMHO, I have committed
> my patch as is and will follow this up on Sunday.

Sounds ok to me.

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] reorder major-0 devices (was Re: [PATCH] implement /proc/sysvipc/*)

[Corinna Vinschen]

On Apr  3 16:54, Yaakov (Cygwin/X) wrote:
> On Fri, 2011-04-01 at 23:33 +0200, Corinna Vinschen wrote:
> > On Apr  1 14:57, Yaakov (Cygwin/X) wrote:
> > > For the sake of clarity, I would reorder it a bit further to
> > > make FH_PROC and friends to one side of major-0 and everything else to
> > > the other side:
> > > 
> > >   /* begin /proc directories */
> > >   FH_PROC= FHDEV (0, 255),
> > >   FH_REGISTRY= FHDEV (0, 254),
> > >   FH_PROCNET = FHDEV (0, 253),
> > >   FH_PROCESSFD = FHDEV (0, 252),
> > >   FH_PROCSYS = FHDEV (0, 251),
> > >   FH_PROCSYSVIPC = FHDEV (0,250),
> > > 
> > >   FH_PROC_MIN_MINOR = FHDEV (0,200),
> > >   /* end /proc directories */
> > > 
> > >   FH_PIPE= FHDEV (0, 199),
> > >   FH_PIPER   = FHDEV (0, 198),
> > >   FH_PIPEW   = FHDEV (0, 197),
> > >   FH_FIFO= FHDEV (0, 196),
> > >   FH_PROCESS = FHDEV (0, 195),
> > >   FH_FS  = FHDEV (0, 194),/* filesystem based device */
> > >   FH_NETDRIVE= FHDEV (0, 193),
> > >   FH_DEV = FHDEV (0, 192),
> > > 
> > > As either way this should be a separate changeset IMHO, I have committed
> > > my patch as is and will follow this up on Sunday.
> > 
> > Sounds ok to me.
> 
> Patch attached.
> 
> 
> Yaakov
> 

> 2011-03-04  Yaakov Selkowitz  <...>
>   Corinna Vinschen  <...>

Bzzz.  Please, don't quote raw email addresses, not even as part of
a patch submission.  Especially don't quote my raw email address.

>   * devices.h (fh_devices): Define FH_PROC_MIN_MINOR.
>   Reorder major-0 devices so that all /proc directories fall
>   between FH_PROC and FH_PROC_MIN_MINOR.
>   * path.h (isproc_dev): Redefine accordingly.

Looks good.  Please apply.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] make compatible with glibc

[Corinna Vinschen]

On Apr  4 01:19, Christopher Faylor wrote:
> On Sun, Apr 03, 2011 at 07:11:51PM -0500, Yaakov (Cygwin/X) wrote:
> >On Sun, 2011-04-03 at 19:55 -0400, Christopher Faylor wrote:
> >> >+#define __INSIDE_CYGWIN_GNU_DEV__
> >> 
> >> I'd prefer a more descriptive name like "__DONT_DEFINE_INLINE_GNU_DEV" 
> >
> >The __INSIDE_CYGWIN_foo__ naming scheme seems to be what is used
> >elsewhere for similar purposes, hence my choice here.
> 
> There is a __INSIDE_CYGWIN_NET__ which I apparently added ten years ago
> but my ideas about naming have changed.  I also added
> USE_SYS_TYPES_FD_SET which is closer to what I now prefer but it should
> have had some leading underscores.

USE_SYS_TYPES_FD_SET is 10 years old, too  ;)

> >> but, then again, why do these have to be exported?  Why can't they just be
> >> always inlined?
> >
> >I just followed what I observed with glibc:
> >
> >$ cat test.c 
> >#include 
> >#include 
> >
> >int
> >main(void)
> >{
> >  int maj = 4, min = 64; /* /dev/ttyS0 */
> >  printf("%d, %d = %d\n", maj, min, makedev(maj, min));
> >  return 0;
> >}
> >
> >$ gcc -O0 test.c
> >
> >$ nm a.out | grep " U "
> > U __libc_start_main@@GLIBC_2.0
> > U gnu_dev_makedev@@GLIBC_2.3.3
> > U printf@@GLIBC_2.0
> >
> >$ gcc -O1 test.c 
> >
> >$ nm a.out | grep " U "
> > U __libc_start_main@@GLIBC_2.0
> > U printf@@GLIBC_2.0
> 
> Maybe the functions were added to gcc before it had the ability to force
> inlining.

Apparently they have been added rather late. Per the man page the macros
existed for a long time, but the exported functions have been added only
with glibc 2.3.3.

> I'll leave it to Corinna but I'd prefer not adding YA export if we can
> avoid it.

This is very simple code, so I, too, would prefer to keep it inline.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] add information to /proc/version

[Corinna Vinschen]

On Apr  3 18:58, Yaakov (Cygwin/X) wrote:
> On Linux, /proc/version also displays the username of the kernel
> compiler and the version of gcc used to compile[1].  This patch does the
> same for Cygwin:
> [...]
>   * new-features.sgml (ov-new1.7.10): Document additional information
>   in /proc/version.

Funny.  Please apply.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] make compatible with glibc

[Corinna Vinschen]

On Apr  4 06:27, Yaakov (Cygwin/X) wrote:
> On Mon, 2011-04-04 at 12:54 +0200, Corinna Vinschen wrote:
> > On Apr  4 01:19, Christopher Faylor wrote:
> > > I'll leave it to Corinna but I'd prefer not adding YA export if we can
> > > avoid it.
> > 
> > This is very simple code, so I, too, would prefer to keep it inline.
> 
> Alright, do I still bump CYGWIN_VERSION_API_MINOR for only inline
> functions?

No, that's not necessary.

> What about posix.sgml?

You can skip it as well.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] make compatible with glibc

[Corinna Vinschen]

On Apr  4 07:41, Yaakov (Cygwin/X) wrote:
>   * include/cygwin/types.h: Move #include  to
>   end of header so the latter get the dev_t typedef.
>   * include/sys/sysmacros.h (gnu_dev_major, gnu_dev_minor,
>   gnu_dev_makedev): Prototype and define as inline functions.
>   (major, minor, makedev): Redefine in terms of gnu_dev_*.

Looks good to me, except for a minor tweak...

> Index: include/cygwin/types.h
> ===
> RCS file: /cvs/src/src/winsup/cygwin/include/cygwin/types.h,v
> retrieving revision 1.33
> diff -u -r1.33 types.h
> --- include/cygwin/types.h29 Mar 2011 10:32:40 -  1.33
> +++ include/cygwin/types.h3 Apr 2011 20:43:20 -
> @@ -17,7 +17,6 @@
>  #ifndef _CYGWIN_TYPES_H
>  #define _CYGWIN_TYPES_H
>  
> -#include 
>  #include 
>  #include 
>  
> @@ -220,6 +219,8 @@
>  #endif /* __INSIDE_CYGWIN__ */
>  #endif /* _CYGWIN_TYPES_H */
>  
> +#include 
> +

...I would move this #include into the _CYGWIN_TYPES_H guarded area,
two lines above.

Oh, btw., while you're at it, would you mind to move the

 #ifdef __cplusplus
 extern "C"
 {
 #endif

and 

 #ifdef __cplusplus
 }
 #endif

into the _CYGWIN_TYPES_H guarded area as well?  I just noticed that for
the first time.  Just a matter of taste I guess, but somehow this looks
upside down to me.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] define _SC_SPIN_LOCKS

[Corinna Vinschen]

On May  2 11:07, Yaakov (Cygwin/X) wrote:
> Corresponding patch to newlib just committed; this is the patch for
> winsup/cygwin/sysconf.cc.
> 
> 
> Yaakov
> 

> 2011-05-02  Yaakov Selkowitz  
> 
>   * sysconf.cc (sca): Set _SC_SPIN_LOCKS to _POSIX_SPIN_LOCKS.

Thanks.  Go ahead.


Corinna


-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] pthread_attr_getstack{,addr}, pthread_getattr_np

[Corinna Vinschen]

On May  2 16:11, Christopher Faylor wrote:
> On Mon, May 02, 2011 at 10:33:09AM -0500, Yaakov (Cygwin/X) wrote:
> >This implements pthread_attr_getstack(), pthread_attr_getstackaddr, and
> >pthread_getattr_np(), which I need for webkitgtk.
> >
> >In essence, I added a stackaddr member to pthread_attr, which is
> >accessed (slightly differently) by pthread_attr_getstack{,attr},
> >behaving just as on Linux.  The bulk of the work is to support
> >pthread_getattr_np, which provides the real attributes of the given
> >thread, including the real stack address and size.
> >
> >The pthread_attr_setstack{,addr} setters are not implemented, as I have
> >yet to find a way to set the thread stack address on Windows.  For that
> >reason I'm not defining _POSIX_THREAD_ATTR_STACKADDR, as the feature is
> >not yet (fully) implemented.
> 
> Cygwin already plays with the stack address.  It has to for situations
> when you call fork() from a thread.

Isn't that what GetThreadContext/SetThreadContext is for?

Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: locale initialization issue

[Corinna Vinschen]

On May  4 07:04, Andy Koppe wrote:
> Hi,
> 
> I stumbled across an issues with locale initialization when the "C"
> locale is specified in the environment.
> [...]
> The attached small patch addresses this by starting with the LC_CTYPE
> locale set to "C.UTF-8"  and lc_ctype_charset set accordingly too.
> This means that setting the "C" locale is recognised as a change and
> that the conversion function pointers are updated accordingly. It also
> has the happy side effect that the setlocale call from
> initial_setlocale() will be short-circuited if the default "C.UTF-8"
> locale has not been overridden in the environment.
> 
> Additionally, I think it's time to drop the "temporarily" #if 0'd code
> for making UTF-8 the charset for the "C" locale.
> 
> It's a newlib patch, but it's entirely Cygwin-specific, so it seemed
> more appropriate to send it here.
> 
>   * libc/locale/locale.c [__CYGWIN__]
>   (current_categories, lc_ctype_charset): Start with the LC_CTYPE locale
>   set to "C.UTF-8", to match initial __wctomb and __mbtowc settings.
>   (lc_message_charset, loadlocale): Settle on ASCII as the "C" charset.

Thanks, applied with a slightly different ChangeLog entry.

Please send newlib patches to the newlib list, not to cygwin-patches.
It's the better place for bookkeeping of newlib stuff, even if it
only affects Cygwin.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] psignal, psiginfo, sys_siglist

[Corinna Vinschen]

On May  4 05:52, Yaakov (Cygwin/X) wrote:
> This patch exports psignal() from newlib (once my corresponding patch is
> accepted) and implements psiginfo() and sys_siglist[].  The first two
> are POSIX.1-2008, the latter is in BSD and glibc.
> 
> Patches for winsup/cygwin and winsup/doc, and a test application,
> attached.
> 
> 
> Yaakov
> 

> 2011-05-04  Yaakov Selkowitz  
> 
>   * cygwin.din (psiginfo): Export.
>   (psignal): Export.
>   (sys_siglist): Export.
>   * posix.sgml (std-notimpl): Move psiginfo and psignal from here...
>   (std-susv4): ... to here.
>   (std-deprec): Add sys_siglist.
>   * strsig.cc (sys_siglist): New array.
>   (psiginfo): New function.
>   * include/cygwin/signal.h (sys_siglist): Declare.
>   (psiginfo): Declare.
>   * include/cygwin/version.h (CYGWIN_VERSION_API_MINOR): Bump.

Looks fine to me.  Chris, what do you think?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] False positive from access("/proc/registry/...", F_OK)

[Corinna Vinschen]

On May  4 22:09, Christian Franke wrote:
>   * fhandler_registry.cc (fhandler_registry::exists): Fix regression
>   in EACCES handling.
>   (fhandler_registry::open): Fix "%val" case.

Applied with the EACCESS typo noted by Eric fixed.


Thanks,
Corinna


-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] update posix.sgml:std-notimpl

[Corinna Vinschen]

On May  4 17:22, Yaakov (Cygwin/X) wrote:
>   * posix.sgml (std-notimpl): Remove bsd_signal, setcontext, and
>   swapcontext, marked obsolete in SUSv3 and not present in SUSv4.

Applied.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] tcsetpgrp fails unexpectedly

[Corinna Vinschen]

On May  5 13:10, Christopher Faylor wrote:
> On Mon, Apr 04, 2011 at 12:23:00PM -0700, Tor Perkins wrote:
> >
> >> Thanks for the patch and the report.  I'll take a look at this in detail
> >> in the next couple of days.  However, unfortunately, I think this is a
> >> large enough submission that it requires an assignment form.
> >
> >Thanks for looking into it!
> >
> >My assignment form is in the "snail".
> 
> Corinna, did you receive this?  Did I miss a notification?

Thanks for the reminder.  The notification was supposed to go directly
to you because I was on vacation at the time.  I'll check.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix access("/proc/registry/HKEY_PERFORMANCE_DATA", R_OK)

[Corinna Vinschen]

On May  5 18:51, Christian Franke wrote:
> This patch fixes access("/proc/registry/HKEY_PERFORMANCE_DATA",
> R_OK) which always fails with EBADF.
> 
> Christian
> 

> 2011-05-05  Christian Franke  <...>
> 
>   * security.cc (check_registry_access): Handle missing
>   security descriptor of HKEY_PERFORMANCE_DATA.

Do you have check in rights?  If so, please check in.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] tcsetpgrp fails unexpectedly

[Corinna Vinschen]

On May  5 19:23, Corinna Vinschen wrote:
> On May  5 13:10, Christopher Faylor wrote:
> > On Mon, Apr 04, 2011 at 12:23:00PM -0700, Tor Perkins wrote:
> > >
> > >> Thanks for the patch and the report.  I'll take a look at this in detail
> > >> in the next couple of days.  However, unfortunately, I think this is a
> > >> large enough submission that it requires an assignment form.
> > >
> > >Thanks for looking into it!
> > >
> > >My assignment form is in the "snail".
> > 
> > Corinna, did you receive this?  Did I miss a notification?
> 
> Thanks for the reminder.  The notification was supposed to go directly
> to you because I was on vacation at the time.  I'll check.

Due to, let's say, "technical" problems I can't answer this question
before Monday.  It seems the CA arrived and was signed, but somebody
has to check.


Sorry,
Corinna


-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix access("/proc/registry/HKEY_PERFORMANCE_DATA", R_OK)

[Corinna Vinschen]

On May  5 23:08, Christian Franke wrote:
> Corinna Vinschen wrote:
> >On May  5 18:51, Christian Franke wrote:
> >>This patch fixes access("/proc/registry/HKEY_PERFORMANCE_DATA",
> >>R_OK) which always fails with EBADF.
> >>
> >>Christian
> >>
> >>2011-05-05  Christian Franke<...>
> >>
> >>* security.cc (check_registry_access): Handle missing
> >>security descriptor of HKEY_PERFORMANCE_DATA.
> >Do you have check in rights?  If so, please check in.
> >
> 
> No check in rights, sorry :-)

http://sourceware.org/cgi-bin/pdw/ps_form.cgi, project Cygwin, approver me.

However, I just had another look into your patch and I have a problem
here.  On what system and with what type of user account did you test?

Here's what I get on Windows 2008 and W7:

$ ~/tests/access /proc/registry/HKEY_PERFORMANCE_DATA
access (/proc/registry/HKEY_PERFORMANCE_DATA, F_OK) = 0
access (/proc/registry/HKEY_PERFORMANCE_DATA, R_OK) = -1 
access (/proc/registry/HKEY_PERFORMANCE_DATA, W_OK) = -1 
access (/proc/registry/HKEY_PERFORMANCE_DATA, X_OK) = -1 

The result is the same on W7 and 2008.  I tried with a normal user
account, as well as with an admin account, with full rights as well as
UAC-restricted.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] sysinfo

[Corinna Vinschen]

On May  6 00:09, Yaakov (Cygwin/X) wrote:
> This implements sysinfo(2), a GNU extension:
> 
> http://www.kernel.org/doc/man-pages/online/pages/man2/sysinfo.2.html
> 
> The code is partially based on our /proc/meminfo and /proc/uptime code.
> (My next patch will port the former to use sysinfo(2), but the latter
> cannot as it uses .01s resolution, more than sysinfo's 1s.  That patch
> will also fix /proc/meminfo and /proc/swaps for RAM and paging files
> larger than 4GB.)
> 
> Patches for winsup/cygwin and winsup/doc, plus a test program, attached.
> 
> 
> Yaakov
> 

> 2011-05-05  Yaakov Selkowitz  
> 
>   * sysconf.cc (sysinfo): New function.
>   * cygwin.din (sysinfo): Export.
>   * posix.sgml (std-gnu): Add sysinfo.
>   * include/sys/sysinfo.h (struct sysinfo): Define.
>   (sysinfo): Declare.
>   * include/cygwin/version.h (CYGWIN_VERSION_API_MINOR): Bump.

That looks good to me.  Just a question...

> +  /* FIXME: unsupported */
> +  info->loads[0] = 0UL;
> +  info->loads[1] = 0UL;
> +  info->loads[2] = 0UL;
> +  info->sharedram = 0UL;
> +  info->bufferram = 0UL;

Isn't bufferram the sum of paged and non-paged pool?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] sysinfo

[Corinna Vinschen]

On May  6 03:51, Yaakov (Cygwin/X) wrote:
> On Fri, 2011-05-06 at 10:11 +0200, Corinna Vinschen wrote:
> > On May  6 00:09, Yaakov (Cygwin/X) wrote:
> > > This implements sysinfo(2), a GNU extension:
> > > 
> > > http://www.kernel.org/doc/man-pages/online/pages/man2/sysinfo.2.html
> > > 
> > > The code is partially based on our /proc/meminfo and /proc/uptime code.
> > > (My next patch will port the former to use sysinfo(2), but the latter
> > > cannot as it uses .01s resolution, more than sysinfo's 1s.  That patch
> > > will also fix /proc/meminfo and /proc/swaps for RAM and paging files
> > > larger than 4GB.)
> > > 
> > > Patches for winsup/cygwin and winsup/doc, plus a test program, attached.
> > > 
> > > 
> > > Yaakov
> > > 
> > 
> > > 2011-05-05  Yaakov Selkowitz  
> > > 
> > >   * sysconf.cc (sysinfo): New function.
> > >   * cygwin.din (sysinfo): Export.
> > >   * posix.sgml (std-gnu): Add sysinfo.
> > >   * include/sys/sysinfo.h (struct sysinfo): Define.
> > >   (sysinfo): Declare.
> > >   * include/cygwin/version.h (CYGWIN_VERSION_API_MINOR): Bump.
> > 
> > That looks good to me.  Just a question...
> > 
> > > +  /* FIXME: unsupported */
> > > +  info->loads[0] = 0UL;
> > > +  info->loads[1] = 0UL;
> > > +  info->loads[2] = 0UL;
> > > +  info->sharedram = 0UL;
> > > +  info->bufferram = 0UL;
> > 
> > Isn't bufferram the sum of paged and non-paged pool?
> 
> The comment alongside the bufferram member of struct sysinfo, as defined
> in the manpage above, says "Memory used by buffers".  A similar meaning
> is given for the Buffers: line of Linux's /proc/meminfo:
> 
> http://docs.redhat.com/docs/en-US/Red_Hat_Enterprise_Linux/6/html/Deployment_Guide/s2-proc-meminfo.html
> 
> So IIUC, no.

I guess you're right.  The pools are used for everything, not only
buffers.

Please check in.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix access("/proc/registry/HKEY_PERFORMANCE_DATA", R_OK)

[Corinna Vinschen]

On May  6 19:28, Christian Franke wrote:
> Corinna Vinschen wrote:
> >On May  5 23:08, Christian Franke wrote:
> >>
> >>No check in rights, sorry :-)
> >http://sourceware.org/cgi-bin/pdw/ps_form.cgi, project Cygwin, approver me.
> >
> 
> Done - Thanks!
> 
> 
> >However, I just had another look into your patch and I have a problem
> >here.  On what system and with what type of user account did you test?
> >
> >Here's what I get on Windows 2008 and W7:
> >
> >$ ~/tests/access /proc/registry/HKEY_PERFORMANCE_DATA
> >access (/proc/registry/HKEY_PERFORMANCE_DATA, F_OK) = 0
> >access (/proc/registry/HKEY_PERFORMANCE_DATA, R_OK) = -1
> >access (/proc/registry/HKEY_PERFORMANCE_DATA, W_OK) = -1 >system>
> >access (/proc/registry/HKEY_PERFORMANCE_DATA, X_OK) = -1
> >
> 
> Hmm
> 
> >The result is the same on W7 and 2008.  I tried with a normal user
> >account, as well as with an admin account, with full rights as well as
> >UAC-restricted.
> >
> >
> 
> Couldn't test new code in Win7 yet.
> 
> Test results:
> 
> Current code on XP SP3 and cygwin-5.7.9-1 on WIn7:
> 
> access("/proc/registry/HKEY_PERFORMANCE_DATA", F_OK)=0
> access("/proc/registry/HKEY_PERFORMANCE_DATA", R_OK)=-1  descriptor>
> access("/proc/registry/HKEY_PERFORMANCE_DATA", W_OK)=-1  file system>
> access("/proc/registry/HKEY_PERFORMANCE_DATA", X_OK)=-1  descriptor>
> 
> Current code + patch on XP
> 
> access("/proc/registry/HKEY_PERFORMANCE_DATA", F_OK)=0
> access("/proc/registry/HKEY_PERFORMANCE_DATA", R_OK)=0
> access("/proc/registry/HKEY_PERFORMANCE_DATA", W_OK)=-1  file system>
> access("/proc/registry/HKEY_PERFORMANCE_DATA", X_OK)=0

Yeah, right.  On second thought that looks much better.  Please
check in.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix /proc/meminfo and /proc/swaps for >4GB

[Corinna Vinschen]

Hi Yaakov,

On May  6 14:03, Yaakov (Cygwin/X) wrote:
> As promised, this patch ports the /proc/meminfo code to use sysinfo(2),
> and fixes the case where RAM or swap space totals more than 4GB.  It
> also fixes the /proc/swaps code for paging files larger than 4GB.
> 
> For example:
> 
> $ cat /proc/meminfo
> total: used: free:
> Mem: 429305856018281512962464907264
> Swap:   12884901888  14680064   12870221824
> MemTotal:4192440 kB
> MemFree: 2407136 kB
> MemShared: 0 kB
> HighTotal: 0 kB
> HighFree:  0 kB
> LowTotal:4192440 kB
> LowFree: 2407136 kB
> SwapTotal:  12582912 kB
> SwapFree:   12568576 kB

I'm not sure I understand this new format.  Why do you keep the Mem: and
Swap: lines?  Linux doesn't have them and top appears to work without
them.  And then, why do you print MemShared, HighTotal, and HighFree,
even though they are always 0, but not all the other ~40 lines Linux'
meminfo has, too?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] clock_settime

[Corinna Vinschen]

Hi Yaakov,

On May  8 17:48, Yaakov (Cygwin/X) wrote:
> This implements the POSIX clock_settime function, on top of settimeofday:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_settime.html
> http://www.kernel.org/doc/man-pages/online/pages/man3/clock_gettime.3.html
> http://www.kernel.org/doc/man-pages/online/pages/man2/settimeofday.2.html
> 
> The fixes to settimeofday are necessary both to match BSD and Linux behaviour,
> and to provide the errnos and return status for clock_settime required by 
> POSIX.
> I also fixed posix.sgml WRT clock_setres.
> 
> Patches for winsup/cygwin and winsup/doc, plus test programs for both
> functions, attached.

Thanks for the patch.

> Index: times.cc
> ===
> RCS file: /cvs/src/src/winsup/cygwin/times.cc,v
> retrieving revision 1.107
> diff -u -r1.107 times.cc
> --- times.cc  2 May 2011 15:28:35 -   1.107
> +++ times.cc  8 May 2011 17:55:34 -
> @@ -111,6 +111,12 @@
>  
>tz = tz;   /* silence warning about unused variable */
>  
> +  if (tv->tv_usec < 0 || tv->tv_usec >= 100)

Not your fault, but what I'm missing in settimeofday is an EFAULT handler.
Could you please add one, just like in the times() function a couple of
lines earlier?  The `tz = tz;' line can go away, the usage is covered
by the syscall_printf at the end of the function.

Other than that, please apply.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] tcsetpgrp fails unexpectedly

[Corinna Vinschen]

On May  5 16:07, Christopher Faylor wrote:
> On Thu, May 05, 2011 at 08:19:24PM +0200, Corinna Vinschen wrote:
> >On May  5 19:23, Corinna Vinschen wrote:
> >> On May  5 13:10, Christopher Faylor wrote:
> >> > On Mon, Apr 04, 2011 at 12:23:00PM -0700, Tor Perkins wrote:
> >> > >
> >> > >> Thanks for the patch and the report.  I'll take a look at this in 
> >> > >> detail
> >> > >> in the next couple of days.  However, unfortunately, I think this is a
> >> > >> large enough submission that it requires an assignment form.
> >> > >
> >> > >Thanks for looking into it!
> >> > >
> >> > >My assignment form is in the "snail".
> >> > 
> >> > Corinna, did you receive this?  Did I miss a notification?
> >> 
> >> Thanks for the reminder.  The notification was supposed to go directly
> >> to you because I was on vacation at the time.  I'll check.
> >
> >Due to, let's say, "technical" problems I can't answer this question
> >before Monday.  It seems the CA arrived and was signed, but somebody
> >has to check.
> 
> Ok.  I'll put this in a cron job to send query email every hour until
> it's resolved.

Resolved.  Tor's copyright assignment made it to Red Hat already weeks
ago.  The guy I asked to keep you informed during my vacation simply
forgot...  Sorry about that.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

Hi Ryan,

On May 11 01:27, Ryan Johnson wrote:
> Hi all,
> 
> Please find attached three patches which extend the functionality of
> /proc/*/maps.

Thanks!

I applied youyr two first patches with a couple of changes:

- Formatting: Setting of curly braces in class and method defintions,
  a lot of missing spaces, "int *foo" instead of "int* foo", stuff
  like that.  Please compare what I checked in against your patch. 
  That doesn't mean I always get it right myself, but basically that's
  how it should be.

- NT_MAX_PATH is the maximum size of a path including the trailing \0,
  so a buffer size of NT_MAX_PATH + 1 isn't required.

- Please always use sys_wcstombs rather than wcstombs for filenames.

- I replaced the call to GetMappedFileNameEx with a call to
  NtQueryVirtualMemory (MemorySectionName).  This avoids to add another
  dependency to psapi.  I intend to get rid of them entirely, if
  possible.

> NOTE 1: I do not attempt to identify PEB, TEB, or thread stacks. The
> first could be done easily enough, but the second and third require
> venturing into undocumented/private Windows APIs.

Go ahead!  We certainly don't shy away from calls to
NtQueryInformationProcess or NtQueryInformationThread.

> NOTE 2: If desired, we could easily extend format_process_maps
> further to report section names of mapped images (linux does this
> for .so files), [...]

Sorry if I'm dense, but isn't that exactly what GetMappedFileNameEx or
NtQueryVirtualMemory (MemorySectionName) do?  I also don't see any extra
information for .so files in the Linux maps output.  What detail am I
missing?


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 11 01:27, Ryan Johnson wrote:
> The second (proc-maps-heaps) adds reporting of Windows heaps (or
> their bases, at least). Unfortunately there doesn't seem to be any
> efficient way to identify all virtual allocations which a heap owns.

There's a call RtlQueryDebugInformation which can fetch detailed heap
information, and which is used by Heap32First/Heap32Last.  Using it
directly is much more efficient than using the Heap32 functions.  The
DEBUG_HEAP_INFORMATION is already in ntdll.h, what's missing is the
layout of the Blocks info.  I found the info by googling:

  typedef struct _HEAP_BLOCK
  {
PVOID addr;
ULONG size;
ULONG flags;
ULONG unknown;
  } HEAP_BLOCK, *PHEAP_BLOCK;

If this information is searched until the address falls into the just
inspected  block of virtual memory, then we would have the information,
isn't it?


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 11 08:53, Ryan Johnson wrote:
> On 11/05/2011 6:31 AM, Corinna Vinschen wrote:
> >- I replaced the call to GetMappedFileNameEx with a call to
> >   NtQueryVirtualMemory (MemorySectionName).  This avoids to add another
> >   dependency to psapi.  I intend to get rid of them entirely, if
> >   possible.
> Nice. One issue: I tried backporting your change to my local tree,
> and the compiler complains that PMEMORY_SECTION_NAME isn't defined;
> the changelog says you updated ntdll.h to add it, but the online
> definition in w32api was last updated 9 months ago by 'duda.' Did it
> perhaps slip past the commit?

The compiler shouldn't complain because we only use the definitions
from ntdll.h and nothing else from w32api/include/ddk (except in rare
cases, see fhandler_tape.cc, fhandler_serial.cc).

Sometimes I apply new stuff to w32api as well on an "as it comes along"
base, but usually it's not the Cygwin project which maintains these
files.

> >>NOTE 1: I do not attempt to identify PEB, TEB, or thread stacks. The
> >>first could be done easily enough, but the second and third require
> >>venturing into undocumented/private Windows APIs.
> >Go ahead!  We certainly don't shy away from calls to
> >NtQueryInformationProcess or NtQueryInformationThread.
> Ah. I didn't realize this sort of thing was encouraged. The MSDN
> docs about them are pretty gloomy.

Gary Nebbett, "Windows NT/2000 Native API Reference".  It's a bit
rusty as far as new stuff is concerned, but it contains the important
stuff we can rely upon.

> The other things that discouraged me before were:
> - the only obvious way to enumerate the threads in a process is to
> create a snapshot using TH32CS_SNAPTHREAD, which enumerates all
> threads in the system. This sounds expensive.
> - it's not clear whether GetThreadContext returns a reasonable stack
> pointer if the target thread is active at the time.

Per MSDN the thread context is only valid if the thread is not running
at the time.   However, we could carefully inspect some of the values
and see how stable they are.

> However, assuming the above do not bother folks, they should be
> pretty straightforward to use. I won't have time to code this up in
> the immediate future, though. My real goal was to make fork bearable
> on my machine and that ended up sucking away all the time I had and
> then some...

Well, it's not high enough on my agenda to dive into it.  Feel free
to do that whenever you find a free time slot.

> >Sorry if I'm dense, but isn't that exactly what GetMappedFileNameEx or
> >NtQueryVirtualMemory (MemorySectionName) do?  I also don't see any extra
> >information for .so files in the Linux maps output.  What detail am I
> >missing?
> Interesting... the machine I used for reference a couple weeks ago
> was running a really old debian, and for each allocation entry of a
> mapped image it gave the corresponding section name (.text, .bss,
> .rdata, etc):
> 346360-346362c000 r-xp  08:01 2097238
> /lib64/libpcre.so.0.0.1 .text
> 346362c000-346382b000 ---p 0002c000 08:01 2097238
> /lib64/libpcre.so.0.0.1
> 346382b000-346382c000 rw-p 0002b000 08:01 2097238
> /lib64/libpcre.so.0.0.1 .bss
> 
> However, the machine was upgraded to a newer kernel this week and
> the extra information no longer appears.

Indeed, this information is not available on my 2.6.35 kernel.

> In any case, should somebody want to report section names within a
> mapped image, that information can be had easily enough using the
> pefile struct from my fork patches.

Right.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 11 13:46, Ryan Johnson wrote:
> On 11/05/2011 7:14 AM, Corinna Vinschen wrote:
> >On May 11 01:27, Ryan Johnson wrote:
> >>The second (proc-maps-heaps) adds reporting of Windows heaps (or
> >>their bases, at least). Unfortunately there doesn't seem to be any
> >>efficient way to identify all virtual allocations which a heap owns.
> >There's a call RtlQueryDebugInformation which can fetch detailed heap
> >information, and which is used by Heap32First/Heap32Last.  Using it
> >directly is much more efficient than using the Heap32 functions.  The
> >DEBUG_HEAP_INFORMATION is already in ntdll.h, what's missing is the
> >layout of the Blocks info.  I found the info by googling:
> >
> >   typedef struct _HEAP_BLOCK
> >   {
> > PVOID addr;
> > ULONG size;
> > ULONG flags;
> > ULONG unknown;
> >   } HEAP_BLOCK, *PHEAP_BLOCK;
> >
> >If this information is searched until the address falls into the just
> >inspected  block of virtual memory, then we would have the information,
> >isn't it?
> The problem is that the reported heap blocks corresponded to
> individual malloc() calls when I tested it (unordered list, most
> sizes < 100B). I really would have preferred a function that returns
> the list of memory regions owned by the heap. They must track that
> information -- I highly HeapDestroy uses these heap blocks to decide
> which memory regions to VirtualFree, but that info seems to live in
> the kernel...

No, it doesn't.  The heap allocation functions are implemented entirely
in user space.  Only virtual memory and shared sections are maintained
by the kernel.

Looking into the MSDN man page for HeapCreate, I'm wondering if this is
really complicated.  HeapCreate just allocates a block of memory using
VirtualAlloc.  Either the Heap is fixed size, then all allocations are
within the given heap memory area, or the heap can grow, then subsequent
(or big-block) allocations can result in another VirtualAlloc.

Having said that, I don't know where this information is stored exactly,
but it must be available in the DEBUG_HEAP_INFORMATION block.  I guess
a bit of experimenting is asked for.

> Given that Heap32* has already been reverse-engineered by others,
> the main challenge would involve sorting the set of heap block
> addresses and distilling them down to a set of allocation bases. We
> don't want to do repeated linear searches over 50k+ heap blocks.

While the base address of the heap is available in
DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
Assuming just that, you could scan the list of blocks once and drop
those within the orignal heap allocation.  The remaining blocks are big
blocks which have been allocated by additional calls to VirtualAlloc.

However, there's a good chance that the entire information about
blocks allocated with VirtualAlloc is kept in the heap's own
datastructures which, again, are undocumented.  I would assume the
start of that heap info is at the heaps base address, but without
more information about the internal structure...

> Also, the cygheap isn't a normal windows heap, is it? I thought it
> was essentially a statically-allocated array (.cygheap) that gets
> managed as a heap. I guess since it's part of cygwin1.dll we already
> do sort of report it...

The cygheap is the last section in the DLL and gets allocated by the
Windows loader.  The memory management is entirely in Cygwin (cygheap.cc).
Cygwin can raise the size of the cygheap, but only if the blocks right
after the existing cygheap are not already allocated.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: [PATCH] Fix /proc/meminfo and /proc/swaps for >4GB

[Corinna Vinschen]

On May 11 22:58, Yaakov (Cygwin/X) wrote:
> On Mon, 2011-05-09 at 09:55 +0200, Corinna Vinschen wrote:
> > I'm not sure I understand this new format.  Why do you keep the Mem: and
> > Swap: lines?  Linux doesn't have them and top appears to work without
> > them.  And then, why do you print MemShared, HighTotal, and HighFree,
> > even though they are always 0, but not all the other ~40 lines Linux'
> > meminfo has, too?
> 
> Actually, my patch makes no attempt to change the actual format
> of /proc/meminfo; it changes only what is necessary to handle RAM or
> swap larger than 4GB by using ULLs instead of ULs.

That's fine and thanks for that.  I'm only puzzled what's actually
printed.

> As for modernizing/fixing the format, true, the Mem: and Swap: lines do
> not exist in modern Linux, nor does the MemShared: line.  I would like
> to actually define at least HighTotal and HighFree; I'll try to look
> into that further soon.  As for the rest of Linux's /proc/meminfo, I'll
> have to see how many other lines can be reasonably determined (if they
> would exist at all) on Windows.

That would be cool, but that wasn't what I meant.  I was just puzzled
that you added values which are always 0, but left out others which are
always 0, too.  I was missing a system, kind of.

> So with the ULL changes, if I remove the Mem, Swap, and MemShared lines,
> will that do for now?

Sure.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 11 21:31, Corinna Vinschen wrote:
> On May 11 13:46, Ryan Johnson wrote:
> > Given that Heap32* has already been reverse-engineered by others,
> > the main challenge would involve sorting the set of heap block
> > addresses and distilling them down to a set of allocation bases. We
> > don't want to do repeated linear searches over 50k+ heap blocks.
> 
> While the base address of the heap is available in
> DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
> the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
> Assuming just that, you could scan the list of blocks once and drop
> those within the orignal heap allocation.  The remaining blocks are big
> blocks which have been allocated by additional calls to VirtualAlloc.

After some debugging, I now have the solution.  If you fetch the heap
and heap block information using the RtlQueryProcessDebugInformation
function, you get the list of blocks.  The first block in the list of
blocks has a flag value of 2.  Let's call it a "start block".  This
block contains the information about the address and size of the virtual
memory block reserved for this heap using VirtualAlloc.

Subsequent blocks do not contain address values, but only size values.
The start of the block is counted relative to the start of the previous
block.  All these blocks are "in-use", up to the last block which has
a flag value of 0x100.  This is the free block at the end of the heap.

For fixed-size heaps, that's all.  Growable heaps can have multiple
memory slots reserved with VirtualAlloc.  For these, if there are still
more blocks, the next block in the list after the free block is a start
block with a flag value of 2 again.  Here's the algorithm which prints
only the virtual memory blocks of all heaps:

  typedef struct _HEAPS
  {
ULONG count;
DEBUG_HEAP_INFORMATION dhi[1];
  } HEAPS, *PHEAPS;

  typedef struct _HEAP_BLOCK
  {
ULONG size;
ULONG flags;
ULONG unknown;
ULONG addr;
  } HEAP_BLOCK, *PHEAP_BLOCK;

  PDEBUG_BUFFER buf;
  NTSTATUS status;

  buf = RtlCreateQueryDebugBuffer (0, FALSE);
  if (!buf)
{
  fprintf (stderr, "RtlCreateQueryDebugBuffer returned NULL\n");
  return 1;
}
  status = RtlQueryProcessDebugInformation (GetCurrentProcessId (),
PDI_HEAPS | PDI_HEAP_BLOCKS,
buf);
  if (!NT_SUCCESS (status))
[...]
  PHEAPS heaps = (PHEAPS) buf->HeapInformation;
  if (!heaps)
[...]
  for (int heap_id = 0; heap_id < heaps->count; ++heap_id)
{
  PHEAP_BLOCK blocks = (PHEAP_BLOCK) heaps->dhi[heap_id].Blocks;
  if (!blocks)
[...]
  uintptr_t addr = 0;
  for (int block_num = 0; block_num < heaps->dhi[heap_id].BlockCount;
   ++block_num)
if (blocks[block_num].flags & 2)
  printf ("addr 0x%08lx, size %8lu"
  blocks[block_num].addr,
blocks[block_num].size);
}
  RtlDestroyQueryDebugBuffer (buf);

As you can imagine, only the blocks I called the start blocks are
of interest for your struct heap_info.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 12 14:10, Corinna Vinschen wrote:
> On May 11 21:31, Corinna Vinschen wrote:
> > On May 11 13:46, Ryan Johnson wrote:
> > > Given that Heap32* has already been reverse-engineered by others,
> > > the main challenge would involve sorting the set of heap block
> > > addresses and distilling them down to a set of allocation bases. We
> > > don't want to do repeated linear searches over 50k+ heap blocks.
> > 
> > While the base address of the heap is available in
> > DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
> > the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
> > Assuming just that, you could scan the list of blocks once and drop
> > those within the orignal heap allocation.  The remaining blocks are big
> > blocks which have been allocated by additional calls to VirtualAlloc.
> 
> After some debugging, I now have the solution. [...]

Here's a prelimiary patch to fhandler_process.cc which takes everything
into account I have learned in the meantime.  For instance, there are
actually heaps marked as shareable.  Please have a look.  What's missing
is the flag for low-fragmentation heaps, but I'm just hunting for it.


Corinna


Index: fhandler_process.cc
===
RCS file: /cvs/src/src/winsup/cygwin/fhandler_process.cc,v
retrieving revision 1.96
diff -u -p -r1.96 fhandler_process.cc
--- fhandler_process.cc 11 May 2011 10:31:22 -  1.96
+++ fhandler_process.cc 12 May 2011 15:08:03 -
@@ -609,39 +609,78 @@ struct dos_drive_mappings
   }
 };
 
+/* Known heap flags */
+#define HEAP_FLAG_NOSERIALIZE  0x1
+#define HEAP_FLAG_GROWABLE 0x2
+#define HEAP_FLAG_EXCEPTIONS   0x4
+#define HEAP_FLAG_NONDEFAULT   0x1000
+#define HEAP_FLAG_SHAREABLE0x8000
+#define HEAP_FLAG_EXECUTABLE   0x4
+
 struct heap_info
 {
   struct heap
   {
 heap *next;
-void *base;
+unsigned heap_id;
+uintptr_t base;
+uintptr_t end;
+unsigned long flags;
   };
   heap *heaps;
 
   heap_info (DWORD pid)
 : heaps (0)
   {
-HANDLE hHeapSnap = CreateToolhelp32Snapshot (TH32CS_SNAPHEAPLIST, pid);
-HEAPLIST32 hl;
-hl.dwSize = sizeof(hl);
-
-if (hHeapSnap != INVALID_HANDLE_VALUE && Heap32ListFirst (hHeapSnap, &hl))
-  do
-   {
- heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
- *h = (heap) {heaps, (void*) hl.th32HeapID};
- heaps = h;
-   } while (Heap32ListNext (hHeapSnap, &hl));
-CloseHandle (hHeapSnap);
+PDEBUG_BUFFER buf;
+NTSTATUS status;
+PDEBUG_HEAP_ARRAY harray;
+
+buf = RtlCreateQueryDebugBuffer (0, FALSE);
+if (!buf)
+  return;
+status = RtlQueryProcessDebugInformation (pid, PDI_HEAPS | PDI_HEAP_BLOCKS,
+ buf);
+if (NT_SUCCESS (status)
+   && (harray = (PDEBUG_HEAP_ARRAY) buf->HeapInformation) != NULL)
+  for (ULONG hcnt = 0; hcnt < harray->Count; ++hcnt)
+   {
+ PDEBUG_HEAP_BLOCK barray = (PDEBUG_HEAP_BLOCK)
+harray->Heaps[hcnt].Blocks;
+ if (!barray)
+   continue;
+ for (ULONG bcnt = 0; bcnt < harray->Heaps[hcnt].BlockCount; ++bcnt)
+   if (barray[bcnt].Flags & 2)
+ {
+   heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
+   *h = (heap) {heaps, hcnt, barray[bcnt].Address,
+barray[bcnt].Address + barray[bcnt].Size,
+harray->Heaps[hcnt].Flags};
+   heaps = h;
+ }
+   }
+RtlDestroyQueryDebugBuffer (buf);
   }
   
-  char *fill_if_match (void *base, char *dest )
+  char *fill_if_match (void *base, ULONG type, char *dest )
   {
-long count = 0;
-for (heap *h = heaps; h && ++count; h = h->next)
-  if (base == h->base)
+for (heap *h = heaps; h; h = h->next)
+  if ((uintptr_t) base >= h->base && (uintptr_t) base < h->end)
{
- __small_sprintf (dest, "[heap %ld]", count);
+ char *p;
+ __small_sprintf (dest, "[heap %ld", h->heap_id);
+ p = strchr (dest, '\0');
+ if (!(h->flags & HEAP_FLAG_NONDEFAULT))
+   p = stpcpy (p, " default");
+ if ((h->flags & HEAP_FLAG_SHAREABLE) && (type & MEM_MAPPED))
+   p = stpcpy (p, " share");
+ if (h->flags & HEAP_FLAG_EXECUTABLE)
+   p = stpcpy (p, " exec");
+ if (h->flags & HEAP_FLAG_GROWABLE)
+   p = stpcpy (p, " grow");
+ if (h->flags & HEAP_FLAG_NOSERIALIZE)
+   p = stpcpy (p, " noserial");
+ stpcpy (p, 

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 12 12:31, Ryan Johnson wrote:
> On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
> >On May 12 14:10, Corinna Vinschen wrote:
> >>On May 11 21:31, Corinna Vinschen wrote:
> >>>On May 11 13:46, Ryan Johnson wrote:
> >>>>Given that Heap32* has already been reverse-engineered by others,
> >>>>the main challenge would involve sorting the set of heap block
> >>>>addresses and distilling them down to a set of allocation bases. We
> >>>>don't want to do repeated linear searches over 50k+ heap blocks.
> >>>While the base address of the heap is available in
> >>>DEBUG_HEAP_INFORMATION, I don't see the size of the heap.  Maybe it's in
> >>>the block of 7 ULONGs marked as "Reserved"?  It must be somewhere.
> >>>Assuming just that, you could scan the list of blocks once and drop
> >>>those within the orignal heap allocation.  The remaining blocks are big
> >>>blocks which have been allocated by additional calls to VirtualAlloc.
> >>After some debugging, I now have the solution. [...]
> >Here's a prelimiary patch to fhandler_process.cc which takes everything
> >into account I have learned in the meantime.  For instance, there are
> >actually heaps marked as shareable.  Please have a look.  What's missing
> >is the flag for low-fragmentation heaps, but I'm just hunting for it.
> I like it. Detailed comments below.
> 
> >+/* Known heap flags */
> >+#define HEAP_FLAG_NOSERIALIZE   0x1
> >+#define HEAP_FLAG_GROWABLE  0x2
> >+#define HEAP_FLAG_EXCEPTIONS0x4
> >+#define HEAP_FLAG_NONDEFAULT0x1000
> >+#define HEAP_FLAG_SHAREABLE 0x8000
> >+#define HEAP_FLAG_EXECUTABLE0x4
> Would it make sense to put those in ntdll.h along with the heap
> structs that use them?

Sure.

> >struct heap
> >{
> >  heap *next;
> >-void *base;
> >+unsigned heap_id;
> >+uintptr_t base;
> >+uintptr_t end;
> >+unsigned long flags;
> >};
> We don't actually need the end pointer: we're trying to match an

No, we need it.  The heaps consist of reserved and committed memory
blocks, as well as of shareable and non-shareable blocks.  Thus you
get multiple VirtualQuery calls per heap, thus you have to check for
the address within the entire heap(*).

> >heap *heaps;
> This is a misnomer now -- it's really a list of heap regions/blocks.

I don't think so.  The loop stores only the blocks which constitute
the original VirtualAlloc'ed memory regions.  They are not the real
heap blocks returned by Heap32First/Heap32Next.  These are filtered
out by checking for flags & 2 (**).

> >+heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
> >+*h = (heap) {heaps, hcnt, barray[bcnt].Address,
> >+ barray[bcnt].Address + barray[bcnt].Size,
> >+ harray->Heaps[hcnt].Flags};
> >+heaps = h;
> Given that the number of heap blocks is potentially large, I think

Not really.  See (**).  3 heaps -> 3 blocks, or only slightly more
if a growable heap got expanded.

> Do you actually encounter requests which fall inside a heap region
> rather than at its start?

Yes, see (*).  And have a look into the output of my code in
contrast to what's printed by the currently version from CVS.

>  I have not seen this in my experiments,
> and if you have it is almost certainly a bug in format_process_maps'
> allocation traversal.

No, see (*).

> Also, are there ever shareable-but-not-mem_mapped segments?

Yes, there are.  2 of the 3 default heaps are marked as shareable, but
one of them is only partially shared.  Of course I don't understand the
reason behind this and how this is accomplished, given that the user
code can't even set a flag "shareable" at HeapCreate time.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 12 12:37, Ryan Johnson wrote:
> On 11/05/2011 3:31 PM, Corinna Vinschen wrote:
> >On May 11 13:46, Ryan Johnson wrote:
> >>Also, the cygheap isn't a normal windows heap, is it? I thought it
> >>was essentially a statically-allocated array (.cygheap) that gets
> >>managed as a heap. I guess since it's part of cygwin1.dll we already
> >>do sort of report it...
> >The cygheap is the last section in the DLL and gets allocated by the
> >Windows loader.  The memory management is entirely in Cygwin (cygheap.cc).
> >Cygwin can raise the size of the cygheap, but only if the blocks right
> >after the existing cygheap are not already allocated.
> Would it make sense to give that section, and the one(s) which
> immediately follow it, the tag "[cygheap]" rather than
> "/usr/bin/cygwin1.dll" and nothing? It would require struct pefile
> to identify the section, plus some trickery in format_process_maps
> to treat the cygwin dll and adjacent succeeding allocation(s)
> specially.

I wouldn't do that.  Just because there's a allocated block right
after the cygheap doesn't mean it's part of the cygheap.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 12 18:55, Corinna Vinschen wrote:
> On May 12 12:31, Ryan Johnson wrote:
> > On 12/05/2011 11:09 AM, Corinna Vinschen wrote:
> > >-void *base;
> > >+unsigned heap_id;
> > >+uintptr_t base;
> > >+uintptr_t end;
> > >+unsigned long flags;
> > >};
> > We don't actually need the end pointer: we're trying to match an
> 
> No, we need it.  The heaps consist of reserved and committed memory
> blocks, as well as of shareable and non-shareable blocks.  Thus you
> get multiple VirtualQuery calls per heap, thus you have to check for
> the address within the entire heap(*).

Btw., here's a good example.  There are three default heaps, One of them,
heap 0, is the heap you get with GetProcessHeap ().  I don't know the
task of heap 1 yet, but heap 2 is ... something, as well as the stack of
the first thread in the process.  It looks like this:

  base 0x0002, flags 0x8000, granularity 8, unknown 0
  allocated 1448, committed65536, block count 3
  Block 0: addr 0x0002, size  2150400, flags 0x0002, unknown 0x0001

However, the various calls to VirtualQuery result in this output with
my patch:

  0002-0003 rw-p  : 0  [heap 2 default share]
  0003-00212000 ---p  : 0  [heap 2 default]
  00212000-00213000 rw-s 001E2000 : 0  [heap 2 default]
  00213000-0023 rw-p 001E3000 : 0  [heap 2 default]

The "something" is the sharable area from 0x2 up to 0x3.  The
stack is from 0x3 up to 0x23.  The first reagion is only
reserved, then the guard page, then the committed and used  tack area.


Corinna


-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat

Re: Extending /proc/*/maps

[Corinna Vinschen]

On May 12 13:53, Ryan Johnson wrote:
> On 12/05/2011 12:55 PM, Corinna Vinschen wrote:
> >>>struct heap
> >>>{
> >>>  heap *next;
> >>>-void *base;
> >>>+unsigned heap_id;
> >>>+uintptr_t base;
> >>>+uintptr_t end;
> >>>+unsigned long flags;
> >>>};
> >>We don't actually need the end pointer: we're trying to match an
> >No, we need it.  The heaps consist of reserved and committed memory
> >blocks, as well as of shareable and non-shareable blocks.  Thus you
> >get multiple VirtualQuery calls per heap, thus you have to check for
> >the address within the entire heap(*).
> The code which queries heap_info already deals with allocations that
> take multiple VirtualQuery calls to traverse, and only calls into
> heap_info for the base of any given allocation. 

However, at least heap 2 consists of multiple allocated memory areas,
which are separately VirtualAlloc'ed or NtMapViewOfSection'ed.  Therefore
the first VirtualQuery returns AllocationBase = BaseAddress = 0x2
and the next VirtualQuery returns AllocationBase = BaseAddress = 0x3.
However, all the memory from 0x2 up to 0x23 constitutes a single
start block in heap 2!

> CAVEAT: According to MSDN's documentation for HeapWalk, "a heap
> consists of one or more regions of virtual memory, each with a
> unique region index." During heap walking, wFlags is set to
> PROCESS_HEAP_REGION whenever "the heap element is located at the
> beginning of a region of contiguous virtual memory in use by the
> heap." However, "large block regions" (those allocated directly via
> VirtualAlloc) do not set the PROCESS_HEAP_REGION flag. If this
> PROCESS_HEAP_REGION flag corresponds to your (flags & 2), then we
> won't report any large blocks (however, it's documented to have the
> value 0x0001, with 0x0002 being PROCESS_HEAP_UNCOMMITTED_RANGE).

I created a test application with several heaps and I create large
blocks in two of them.  They also have the flags&2 value set.  The
problem for heap walk is to identify blocks with a valid address.  Keep
in mind that all subsequent blocks within an allocated heap area do
*not* have an addres, but only a size, with address == 0.  You only get
their address by iterating from the start block to the block you're
looking for and adding up all sizes of the blocks up to there.

Consequentially a start block of another VirtualAlloc'ed area needs a
marker that the address is valid.  That marker is the flags value 2.
Which is kind of weird, actually, since the existence of a non-0 address
in the block should have been enough of a hint that this is a start
block...

As for the big blocks, they are apparently identified by the value in
the "Unknown" member of the DEBUG_HEAP_BLOCK structure.  Here's what I
figured out so far as far as "Unknown" is concerned:

   0x1000  All normal heaps
   0x3000  The process default heap (heap 0)
   0xc000  A low-frag heap
  0x1  Heap 2, perhaps the meaning is "stack"?
  0x32000  Subsequently allocated block of a growable heap
 0x1e9000  Large block

I don't claim to understand the values, especially the reason for
setting several bits.

> >>>heap *heaps;
> >>This is a misnomer now -- it's really a list of heap regions/blocks.
> >I don't think so.  The loop stores only the blocks which constitute
> >the original VirtualAlloc'ed memory regions.  They are not the real
> >heap blocks returned by Heap32First/Heap32Next.  These are filtered
> >out by checking for flags&  2 (**).
> Sorry, I cut too much context out of the diff. That's
> heap_info::heaps, which indeed refers to heap regions which we
> identified by checking flags&2 (that's why it needs the heap_id
> inside it now, where it didn't before) (++)

So you think something like heap_chunks is better?

> >>>+  heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
> >>>+  *h = (heap) {heaps, hcnt, barray[bcnt].Address,
> >>>+   barray[bcnt].Address + barray[bcnt].Size,
> >>>+   harray->Heaps[hcnt].Flags};
> >>>+  heaps = h;
> >>Given that the number of heap blocks is potentially large, I think
> >Not really.  See (**).  3 heaps ->  3 blocks, or only slightly more
> >if a growable heap got expanded.
> See (++). When I point my essentially-identical version of the code
> at emacs, it reports 8 heaps, each with 2-4 regions. The list
> traversed by fill_on_match has ~20 entries.

Oh, ok.  From my POV 20 is not a large number.  Ordering might take
more time than scanning.  I don't think it's worth the effort.


Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader  cygwin AT cygwin DOT com
Red Hat