August 17, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | Basically because you want to know what you're piping to another process. Say you read a file up to some point, and want to pass the rest of it through a pipe. That won't work if you use File, because you don't know what has already been buffered, and what's left in the stream. I was originally using File, but after Steve pointed out the problem above we decided to add an UnbufferedFile type. Hopefully, it will find other uses as well -- perhaps, at some point, we can even build a new buffered File on top of it. The current API for UnbufferedFile is here, if anyone is wondering: http://www.kyllingen.net/code/ltk/doc/stdio.html -Lars On Mon, 2010-08-16 at 12:20 -0500, Andrei Alexandrescu wrote: > I also like ltk process quite a bit. One q - why the need for a special type UnbufferedFile? > > Andrei > > David Simcha wrote: > > This looks terrific. I've always found the old std.process to be way underpowered, especially on Windows. Does your statement about cross-platformness imply that Windows will eventually be supported, too? > > > > On Mon, Aug 16, 2010 at 9:20 AM, Lars Tandle Kyllingstad <lars at kyllingen.net <mailto:lars at kyllingen.net>> wrote: > > > > On Mon, 2010-08-16 at 09:04 -0400, Adam Ruppe wrote: > > > I actually use it (which is why I duplicated your bug), but am OK > > with > > > removing it, since it is easy enough to get at anyway. For a while, I > > > did a separate extern(C) for pclose anyway! > > > > > > However, I don't think something being POSIX only is a good reason to > > > remove something. D should take advantages of whatever platform it is > > > on. Portability is good when you can have it, but it shouldn't be a > > > function killer alone. > > > > Two comments: > > > > 1. I disagree with you. :) I think that Phobos' user-visible interface > > should be completely platform agnostic. Code that depends only on > > Phobos should compile and run on any platform. > > > > 2. Steve and I have been working on a new version of std.process, which > > will at some point, hopefully, obviate the need for popen(). See > > pipeProcess() here: > > > > http://www.kyllingen.net/code/ltk/doc/process.html > > > > The POSIX implementation is more or less complete, but its inclusion in > > Phobos is currently being blocked by bug 3979. Also, Steve has run into > > some very tricky issues with pipes on Windows, fundamentally caused by > > D's dependence on the DMC runtime. I don't know how (or if) that is > > working out. > > > > -Lars > > > > _______________________________________________ > > phobos mailing list > > phobos at puremagic.com <mailto:phobos at puremagic.com> > > http://lists.puremagic.com/mailman/listinfo/phobos > > > > > > > > ------------------------------------------------------------------------ > > > > _______________________________________________ > > phobos mailing list > > phobos at puremagic.com > > http://lists.puremagic.com/mailman/listinfo/phobos > _______________________________________________ > phobos mailing list > phobos at puremagic.com > http://lists.puremagic.com/mailman/listinfo/phobos |
August 17, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Shin Fujishiro | On Tue, 2010-08-17 at 01:53 +0900, Shin Fujishiro wrote: > Lars Tandle Kyllingstad <lars at kyllingen.net> wrote: > > 2. Steve and I have been working on a new version of std.process, which > > will at some point, hopefully, obviate the need for popen(). See > > pipeProcess() here: > > > > http://www.kyllingen.net/code/ltk/doc/process.html > > I'm looking forward to seeing the result! :-) Me too. :) > > The POSIX implementation is more or less complete, but its inclusion in Phobos is currently being blocked by bug 3979. Also, Steve has run into some very tricky issues with pipes on Windows, fundamentally caused by D's dependence on the DMC runtime. I don't know how (or if) that is working out. > > Will the new std.process use C stdio? No, it uses the OS file descriptors/handles directly. -Lars |
August 17, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars Tandle Kyllingstad | I understand that, and the motivation has been thoroughly explained in the Unix lore. My question is, why define a separate type? Wouldn't creating a file and calling setbuf(null, 0) suffice?
Andrei
On 08/17/2010 01:59 AM, Lars Tandle Kyllingstad wrote:
> Basically because you want to know what you're piping to another process. Say you read a file up to some point, and want to pass the rest of it through a pipe. That won't work if you use File, because you don't know what has already been buffered, and what's left in the stream.
>
> I was originally using File, but after Steve pointed out the problem above we decided to add an UnbufferedFile type. Hopefully, it will find other uses as well -- perhaps, at some point, we can even build a new buffered File on top of it.
>
> The current API for UnbufferedFile is here, if anyone is wondering:
>
> http://www.kyllingen.net/code/ltk/doc/stdio.html
>
> -Lars
>
>
>
> On Mon, 2010-08-16 at 12:20 -0500, Andrei Alexandrescu wrote:
>> I also like ltk process quite a bit. One q - why the need for a special type UnbufferedFile?
>>
>> Andrei
>>
>> David Simcha wrote:
>>> This looks terrific. I've always found the old std.process to be way underpowered, especially on Windows. Does your statement about cross-platformness imply that Windows will eventually be supported, too?
>>>
>>> On Mon, Aug 16, 2010 at 9:20 AM, Lars Tandle Kyllingstad <lars at kyllingen.net<mailto:lars at kyllingen.net>> wrote:
>>>
>>> On Mon, 2010-08-16 at 09:04 -0400, Adam Ruppe wrote:
>>> > I actually use it (which is why I duplicated your bug), but am OK
>>> with
>>> > removing it, since it is easy enough to get at anyway. For a while, I
>>> > did a separate extern(C) for pclose anyway!
>>> >
>>> > However, I don't think something being POSIX only is a good reason to
>>> > remove something. D should take advantages of whatever platform it is
>>> > on. Portability is good when you can have it, but it shouldn't be a
>>> > function killer alone.
>>>
>>> Two comments:
>>>
>>> 1. I disagree with you. :) I think that Phobos' user-visible interface
>>> should be completely platform agnostic. Code that depends only on
>>> Phobos should compile and run on any platform.
>>>
>>> 2. Steve and I have been working on a new version of std.process, which
>>> will at some point, hopefully, obviate the need for popen(). See
>>> pipeProcess() here:
>>>
>>> http://www.kyllingen.net/code/ltk/doc/process.html
>>>
>>> The POSIX implementation is more or less complete, but its inclusion in
>>> Phobos is currently being blocked by bug 3979. Also, Steve has run into
>>> some very tricky issues with pipes on Windows, fundamentally caused by
>>> D's dependence on the DMC runtime. I don't know how (or if) that is
>>> working out.
>>>
>>> -Lars
>>>
>>> _______________________________________________
>>> phobos mailing list
>>> phobos at puremagic.com<mailto:phobos at puremagic.com>
>>> http://lists.puremagic.com/mailman/listinfo/phobos
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> _______________________________________________
>>> phobos mailing list
>>> phobos at puremagic.com
>>> http://lists.puremagic.com/mailman/listinfo/phobos
>> _______________________________________________
>> phobos mailing list
>> phobos at puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/phobos
>
>
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
|
August 17, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars Tandle Kyllingstad | It doesn't matter how often it's used internally, but instead how often it's used by programs. I use it and find it very useful. We should move toward fixing it and porting it to other OSs. Replacing with the more comprehensive solution that's being discussed is of course another option. Incidentally a colleague of mine mentioned today a much faster implementation of popen(): http://blog.famzah.net/2009/11/20/a-much-faster-popen-and-system-implementation-for-linux/ http://code.google.com/p/popen-noshell/ We should get that in Phobos. Andrei On 08/16/2010 03:11 AM, Lars Tandle Kyllingstad wrote: > I just noticed that std.stdio.File.popen() is POSIX-only. Taking that > into account, along with the fact that it is buggy (issue 3157), does > anyone mind if I remove it? > > It's only used once in Phobos, namely in std.process.shell(), which is > easily implemented by using core.sys.posix.stdio.popen() together with > core.sys.posix.stdio.fdopen() and std.stdio.File.wrapFile(). > > -Lars > > _______________________________________________ > phobos mailing list > phobos at puremagic.com > http://lists.puremagic.com/mailman/listinfo/phobos |
August 18, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Tue, 2010-08-17 at 22:03 -0500, Andrei Alexandrescu wrote: > I understand that, and the motivation has been thoroughly explained in the Unix lore. My question is, why define a separate type? Wouldn't creating a file and calling setbuf(null, 0) suffice? Well, for one thing, I didn't know about setbuf() until just now. :) I still think I prefer the UnbufferedFile solution, though. setbuf() should be called before anything is read or written to the stream. This means it will be up to the user to do so, and this is easily forgotten. Compare auto f = UnbufferedFile("myfile.txt"); ... auto p = spawnProcess("myapp", f); to auto f = File("myfile.txt"); setbuf(f.getFP(), null); // DON'T FORGET THIS! ... auto p = spawnProcess("myapp", f); An alternative solution is to have the user specify whether File should be buffered or unbuffered upon construction. Then spawnProcess() can check this and throw an exception if the File is buffered. auto f = File("myfile.txt", Buffer.none); ... auto p = spawnProcess("myapp", f); // succeeds You mentioned a few months ago that you were considering replacing/complementing the stdioOpenmode string argument to File's constructor with an enum. Perhaps buffered-ness could also be an option here? auto f = File("myfile.txt", Mode.append | Mode.noBuffer); -Lars On Tue, 2010-08-17 at 22:03 -0500, Andrei Alexandrescu wrote: > I understand that, and the motivation has been thoroughly explained in the Unix lore. My question is, why define a separate type? Wouldn't creating a file and calling setbuf(null, 0) suffice? > > Andrei > > On 08/17/2010 01:59 AM, Lars Tandle Kyllingstad wrote: > > Basically because you want to know what you're piping to another process. Say you read a file up to some point, and want to pass the rest of it through a pipe. That won't work if you use File, because you don't know what has already been buffered, and what's left in the stream. > > > > I was originally using File, but after Steve pointed out the problem above we decided to add an UnbufferedFile type. Hopefully, it will find other uses as well -- perhaps, at some point, we can even build a new buffered File on top of it. > > > > The current API for UnbufferedFile is here, if anyone is wondering: > > > > http://www.kyllingen.net/code/ltk/doc/stdio.html > > > > -Lars > > > > > > > > On Mon, 2010-08-16 at 12:20 -0500, Andrei Alexandrescu wrote: > >> I also like ltk process quite a bit. One q - why the need for a special type UnbufferedFile? > >> > >> Andrei > >> > >> David Simcha wrote: > >>> This looks terrific. I've always found the old std.process to be way underpowered, especially on Windows. Does your statement about cross-platformness imply that Windows will eventually be supported, too? > >>> > >>> On Mon, Aug 16, 2010 at 9:20 AM, Lars Tandle Kyllingstad <lars at kyllingen.net<mailto:lars at kyllingen.net>> wrote: > >>> > >>> On Mon, 2010-08-16 at 09:04 -0400, Adam Ruppe wrote: > >>> > I actually use it (which is why I duplicated your bug), but am OK > >>> with > >>> > removing it, since it is easy enough to get at anyway. For a while, I > >>> > did a separate extern(C) for pclose anyway! > >>> > > >>> > However, I don't think something being POSIX only is a good reason to > >>> > remove something. D should take advantages of whatever platform it is > >>> > on. Portability is good when you can have it, but it shouldn't be a > >>> > function killer alone. > >>> > >>> Two comments: > >>> > >>> 1. I disagree with you. :) I think that Phobos' user-visible interface > >>> should be completely platform agnostic. Code that depends only on > >>> Phobos should compile and run on any platform. > >>> > >>> 2. Steve and I have been working on a new version of std.process, which > >>> will at some point, hopefully, obviate the need for popen(). See > >>> pipeProcess() here: > >>> > >>> http://www.kyllingen.net/code/ltk/doc/process.html > >>> > >>> The POSIX implementation is more or less complete, but its inclusion in > >>> Phobos is currently being blocked by bug 3979. Also, Steve has run into > >>> some very tricky issues with pipes on Windows, fundamentally caused by > >>> D's dependence on the DMC runtime. I don't know how (or if) that is > >>> working out. > >>> > >>> -Lars > >>> > >>> _______________________________________________ > >>> phobos mailing list > >>> phobos at puremagic.com<mailto:phobos at puremagic.com> > >>> http://lists.puremagic.com/mailman/listinfo/phobos > >>> > >>> > >>> > >>> ------------------------------------------------------------------------ > >>> > >>> _______________________________________________ > >>> phobos mailing list > >>> phobos at puremagic.com > >>> http://lists.puremagic.com/mailman/listinfo/phobos > >> _______________________________________________ > >> phobos mailing list > >> phobos at puremagic.com > >> http://lists.puremagic.com/mailman/listinfo/phobos > > > > > > _______________________________________________ > > phobos mailing list > > phobos at puremagic.com > > http://lists.puremagic.com/mailman/listinfo/phobos |
August 18, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Tue, 2010-08-17 at 23:46 -0500, Andrei Alexandrescu wrote: > It doesn't matter how often it's used internally, but instead how often it's used by programs. I use it and find it very useful. We should move toward fixing it and porting it to other OSs. Replacing with the more comprehensive solution that's being discussed is of course another option. Not only is it an option, it seems to be a lot more imminent than I thought. If we can get Rainer's patch for bug 3979 into the next release, I'm guessing we can have a revamped std.process by the release following that. > Incidentally a colleague of mine mentioned today a much faster implementation of popen(): > > http://blog.famzah.net/2009/11/20/a-much-faster-popen-and-system-implementation-for-linux/ > > http://code.google.com/p/popen-noshell/ > > We should get that in Phobos. This is very useful information! I'll see if this can be incorporated in the new std.process. -Lars |
August 18, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars Tandle Kyllingstad | On Wed, 2010-08-18 at 10:17 +0200, Lars Tandle Kyllingstad wrote:
> On Tue, 2010-08-17 at 23:46 -0500, Andrei Alexandrescu wrote:
> > Incidentally a colleague of mine mentioned today a much faster implementation of popen():
> >
> > http://blog.famzah.net/2009/11/20/a-much-faster-popen-and-system-implementation-for-linux/
> >
> > http://code.google.com/p/popen-noshell/
> >
> > We should get that in Phobos.
>
> This is very useful information! I'll see if this can be incorporated in the new std.process.
I had a more thorough look at it now, and from what I can understand, this trick trades safety for speed.
Instead of fork(), it calls clone(). This allows the child to run in the same memory space as the parent (but with a separate stack), which avoids the overhead of duplicating the parent's memory. This is all well and good if you're sure you can trust the program you're calling to be both bug-free and non-malicious, but in general I would be reluctant to do so.
There is of course a good chance that I may be missing some part of the trick that he's not mentioning in the article. The code itself is GPL'ed, so I don't want to look at it. Can anyone here think of a way to make this safe?
-Lars
|
August 18, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars Tandle Kyllingstad | On Wed, 2010-08-18 at 11:10 +0200, Lars Tandle Kyllingstad wrote:
> On Wed, 2010-08-18 at 10:17 +0200, Lars Tandle Kyllingstad wrote:
> > On Tue, 2010-08-17 at 23:46 -0500, Andrei Alexandrescu wrote:
> > > Incidentally a colleague of mine mentioned today a much faster implementation of popen():
> > >
> > > http://blog.famzah.net/2009/11/20/a-much-faster-popen-and-system-implementation-for-linux/
> > >
> > > http://code.google.com/p/popen-noshell/
> > >
> > > We should get that in Phobos.
> >
> > This is very useful information! I'll see if this can be incorporated in the new std.process.
>
> I had a more thorough look at it now, and from what I can understand, this trick trades safety for speed.
Forget it. execve() takes care of creating a new memory space. I'll definitely add this to std.process.
-Lars
|
August 18, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars Tandle Kyllingstad | On 08/18/2010 03:10 AM, Lars Tandle Kyllingstad wrote: > On Tue, 2010-08-17 at 22:03 -0500, Andrei Alexandrescu wrote: >> I understand that, and the motivation has been thoroughly explained in the Unix lore. My question is, why define a separate type? Wouldn't creating a file and calling setbuf(null, 0) suffice? > > > Well, for one thing, I didn't know about setbuf() until just now. :) > > I still think I prefer the UnbufferedFile solution, though. setbuf() should be called before anything is read or written to the stream. This means it will be up to the user to do so, and this is easily forgotten. Compare > > auto f = UnbufferedFile("myfile.txt"); > ... > auto p = spawnProcess("myapp", f); > > to > > auto f = File("myfile.txt"); > setbuf(f.getFP(), null); // DON'T FORGET THIS! > ... > auto p = spawnProcess("myapp", f); spawnProcess could call setbuf itself and fail if that doesn't go through. > An alternative solution is to have the user specify whether File should be buffered or unbuffered upon construction. Then spawnProcess() can check this and throw an exception if the File is buffered. > > auto f = File("myfile.txt", Buffer.none); > ... > auto p = spawnProcess("myapp", f); // succeeds As said above, I think CRT's setvbuf fails if called twice. > You mentioned a few months ago that you were considering replacing/complementing the stdioOpenmode string argument to File's constructor with an enum. Perhaps buffered-ness could also be an option here? > > auto f = File("myfile.txt", Mode.append | Mode.noBuffer); If needed, sure. Yet if buffering isn't needed the old-style modes put in a string seem to be adequate and are documented everywhere. Andrei |
August 18, 2010 [phobos] Removing std.stdio.File.popen() | ||||
---|---|---|---|---|
| ||||
Posted in reply to Lars Tandle Kyllingstad | On 08/18/2010 04:10 AM, Lars Tandle Kyllingstad wrote:
> On Wed, 2010-08-18 at 10:17 +0200, Lars Tandle Kyllingstad wrote:
>> On Tue, 2010-08-17 at 23:46 -0500, Andrei Alexandrescu wrote:
>>> Incidentally a colleague of mine mentioned today a much faster
>>> implementation of popen():
>>>
>>> http://blog.famzah.net/2009/11/20/a-much-faster-popen-and-system-implementation-for-linux/
>>>
>>> http://code.google.com/p/popen-noshell/
>>>
>>> We should get that in Phobos.
>>
>> This is very useful information! I'll see if this can be incorporated in the new std.process.
>
> I had a more thorough look at it now, and from what I can understand, this trick trades safety for speed.
>
> Instead of fork(), it calls clone(). This allows the child to run in the same memory space as the parent (but with a separate stack), which avoids the overhead of duplicating the parent's memory. This is all well and good if you're sure you can trust the program you're calling to be both bug-free and non-malicious, but in general I would be reluctant to do so.
>
> There is of course a good chance that I may be missing some part of the trick that he's not mentioning in the article. The code itself is GPL'ed, so I don't want to look at it. Can anyone here think of a way to make this safe?
Doesn't he call exec() immediately after clone()?
Andrei
|
Copyright © 1999-2021 by the D Language Foundation