Skip to content

Commit

Permalink
Actually no need for Horde_Stream at all.
Browse files Browse the repository at this point in the history
  • Loading branch information
@yunosh
yunosh committed Feb 3, 2017
1 parent 18c8fe3 commit baa79ba
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 10 deletions.
6 changes: 3 additions & 3 deletions framework/Vfs/lib/Horde/Vfs/Base.php
View file
Expand Up @@ -801,9 +801,9 @@ protected function _ensureSeekable($stream)

$meta = stream_get_meta_data($stream);
if (!$meta['seekable']) {
$temp = new Horde_Stream_Temp();
$temp->add($stream, true);
return $temp->stream;
$temp = @fopen('php://temp', 'r+');
stream_copy_to_stream($stream, $temp);
return $temp;
}

This comment has been minimized.

Sign in to view

Copy link
@mrubinsk

mrubinsk Feb 3, 2017

Member

I tend to use Horde_Stream when doing operations like this since it explicitly doesn't use stream_copy_to_stream, but copies the stream in smaller chunks, and quite frankly it's easier to reuse that code instead of copying it.

From what I understand stream_copy_to_stream has (or at least had) some memory/performance issues?

This comment has been minimized.

Sign in to view

Copy link
@yunosh

yunosh Feb 14, 2017

Author Member

Chunks is a good point, but we shouldn't introduce a whole dependency for such a small use case. We even only use a tiny fraction of Horde_Stream::add()'s code, let alone Horde_Stream's altogether.

rewind($stream);
Expand Down
7 changes: 0 additions & 7 deletions framework/Vfs/package.xml
View file
Expand Up @@ -405,13 +405,6 @@ Reading, writing and listing of files are all supported.</description>
<max>3.0.0alpha1</max>
<exclude>3.0.0alpha1</exclude>
</package>
<package>
<name>Horde_Stream</name>
<channel>pear.horde.org</channel>
<min>2.0.0</min>
<max>3.0.0alpha1</max>
<exclude>3.0.0alpha1</exclude>
</package>
<package>
<name>Horde_Translation</name>
<channel>pear.horde.org</channel>
Expand Down

0 comments on commit baa79ba

Please sign in to comment.