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

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

RE: ksh on cygwin

> > It's a whole bunch of small fixes. I think I need to fill 
> out the assignment form.
> Yeah, please send it as soon as possible since you'll have to send
> it by snail mail.  Sometimes it takes two to three weeks for some
> reason.

OK, I'll fill it out later today.

> > Is it OK to send patches to 1.3.3-2 or should I move them 
> to 1.3.6 first?
> I would suggest to move them to the latest from CVS.  If you're
> always working against the latest from CVS you don't get hit too
> much by changes from other people.

I'll try that.

> > I'll give a short summary of what I've fixed:
> I'll compare your below listing against current from CVS.  You should
> evaluate my answers in that light.
> > - pathconf() doesn't check existance of the path
> It does.  However, there's an error in _PC_PATH_MAX.   It doesn't
> validate the incoming path which could result in a SEGV.  I've
> checked in a fix.


> > - getpagesize() should return a value compatible with 
> mmap(), that is dwAllocGranularity (65536) instead of 
> dwPageSize (1024).
> We discussed that months ago.  I think we're not going to change that
> (it's 4096, not 1024, btw.).

Oops, I was writing this summary from memory, I don't have the patches at hand now...

> It will result in dubious problems
> when a process mmaps a file.  For instance, the latest gcc expects to
> be able to read over the end of an mmaped file if the size is not a
> multiple of getpagesize().  Now think of a file which is 
> coincidentally
> exactly 1 page long...

Glenn found some test cases where mmap() failed and has also written a nice test program. I will get this to you later.
He also states that the value returned by getpagesize() must conform to mmap() alignment by definition in the SUSv2. I'm not quite sure about that, though.

> > - use .exe extension detection consequently in all syscalls
> You mean unlink() etc.?

Yes. chmod(), link(), open(), pathconf(), rename(), truncate() and unlink() (have I missed one?) didn't check for the .exe extension.

> > - automatically add the .exe extension to a newly created 
> file on close() when the first four bytes are 0x4d, 0x5a, 
> 0x90, 0x00 (MS Executable magic bytes) and the file name 
> didn't have an extension already. (This is from UWIN, I think 
> it's really nice).
> Ugh.  I'm not quite sure if I like that.

This eliminates the need to fix up programs which produce executables, e.g. ld.
Also you don't need to take care of the .exe extension in cp.
It's there automatically.

> > - exec*() functions would always invoke a /bin/sh on a file 
> that isn't a valid executable. Only execlp()/execvp() should 
> do so, others must return with an ENOEXEC.
> Sounds like a bug.

Yes. A big one.

> > - the exec*() fix revealed a bug with vfork() in ash
> We have some vfork() changes in the meantime and even ash had an 
> related error which should be fixed.

Maybe we fixed the same error. I'll send you the details.

> > - use the contents of $SHELL instead of /bin/sh for 
> execvp()/execlp() and system() (with some additional checks, 
> e.g. do not use a csh, use only 'trusted' shells from /bin, 
> /usr/bin, /usr/local/bin etc.). This allows the user to 
> select his favorite shell manually, so no more "copy 
> /bin/bash to /bin/sh" troubles. (This is also from UWIN).
> Hmm, interesting idea...

OK, more detailed. I allow only absolute pathes in $SHELL and don't allow any *csh.
If superuser then only shells from [/usr][/local]/bin are considered trusted shells.
If not superuser shells from other directories are allowed, but if uid != euid or gid != egid the shell and the directory where it resides must not be writable.
Fall back value is /bin/sh.

> > - some mmap() problems have been fixed.
> Since I have changed so much in mmap() you should actually first
> compare your changes to the current version.

I'll do that.

> > - utime() doesn't mark st_ctime for update
> Really?  I would never think so when inspecting the source code.

Has this been fixed meanwhile? Also other calls like chmod() must mark st_ctime for update. My patches are not complete here.


Unsubscribe info:
Bug reporting:

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