Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Make sure we return complete lines from readline() and fix regressions #944

Open
wants to merge 11 commits into from

5 participants

Gerhard R. Jimmy Zhuo Duke Leto timo Reini Urban
Gerhard R.

This should fix everything that was broken by the premature merge of my branch, as well as implementing the readline() change.

gerdr added some commits
Gerhard R. gerdr Make readline() always return complete lines instead of partial ones,…
… even if we have to block
c01a0a5
Gerhard R. gerdr adjust FIXME comment e084eba
Gerhard R. gerdr Next shot at fixing io_readline_encoded_string().
All regression should be gone and HTTP::Easy still works.
I also identified a potential problem already present in the
original code.
243bad2
Gerhard R. gerdr Purge set_eof from IO subsystem.
Being at EOF is something the underlying system handle keeps track of, and not
something we should be able to affect from userspace.
7fac981
Jimmy Zhuo
Collaborator

If nobody deals with this pull request in the next three days, I will merge it. :)

Gerhard R.

Wow. That ended up being slightly more work that I set out to do.

Please wait with the merge to master until others have had time to review these changes.

In particular, the following semantics have changed:

  • get_bool(), eof() and is_open() share a single method implementation in the Handle superclass
  • eof() on sockets does something useful now: it checks if the last buffered read succeeded (or rather, didn't)
  • get_bool() checks if the handle is open instead of looking for EOF, which is imo the more useful behaviour for generic handle types, which necessitated changing a single test
  • the set_eof() IO vtable function is gone - it was a noop for most types and is an implementation detail that shouldn't be exposed

Stuff I still need to work on:

  • align io_read_encoded_string() with io_readline_encoded_string()
  • add tests for readline() with sockets, which should now work as expected for all edge cases I could come up with

Stuff to think about:

  • move flags from FileHandle and StringHandle to Handle, use PIO_F_EOF instead of PIO_BF_UNDERFLOW for sockets
  • generic implementations for IO vtable functions that can be shared
Gerhard R.

I'm taking a shot at the 'stuff to think about' in the branch https://github.com/gerdr/parrot/commits/io-refactor

Duke Leto
Owner

What is the status of this? It is not clear to me what needs to happen to close this issue. Can somebody update the description with details?

Gerhard R.

The pull request as-is could in principle be merged: It test cleanly and fixes #942.

However, there's still some more stuff I want to do, in particular aligning io_read_encoded_string() with the new io_readline_encoded_string() semantics and adding tests.

I don't particularly like the factoring, though, which is why I started https://github.com/gerdr/parrot/commits/io-refactor (which right now breaks GzipHandle but otherwise passes the tests as well).

I'll try to get back to this sooner rather than later, but no guarantees...

Duke Leto
Owner

Can you please give a status update about this, @gerdr ? Thank you!

timo

hey @gerdr, there's still a pull request open for rakudo that references this bug (rakudo/rakudo#107).
Is the branch still interesting?

Reini Urban
Collaborator

I fixed up this branch in gerdr/socket-readline-gh944.
One blocker though: parrot_nci_thunk_gen hangs after having written the buffer. It could be the read from the pipe missing it's eof.

./parrot_nci_thunk_gen ... --output=src/glut_nci_thunks.c < src/glut_nci_thunks.nci

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 12, 2013
  1. Gerhard R.

    Make readline() always return complete lines instead of partial ones,…

    gerdr authored
    … even if we have to block
  2. Gerhard R.

    adjust FIXME comment

    gerdr authored
  3. Gerhard R.

    Next shot at fixing io_readline_encoded_string().

    gerdr authored
    All regression should be gone and HTTP::Easy still works.
    I also identified a potential problem already present in the
    original code.
  4. Gerhard R.

    Purge set_eof from IO subsystem.

    gerdr authored
    Being at EOF is something the underlying system handle keeps track of, and not
    something we should be able to affect from userspace.
Commits on Mar 13, 2013
  1. Gerhard R.
  2. Gerhard R.
  3. Gerhard R.
  4. Gerhard R.
  5. Gerhard R.

    Fix bug in Parrot_io_readline_s()

    gerdr authored
    io_verify_has_read_buffer() expects a flags argument, not the buffer size
  6. Gerhard R.
  7. Gerhard R.
This page is out of date. Refresh to see the latest.
12 include/parrot/io.h
View
@@ -32,10 +32,11 @@
#endif
/* Buffer flags */
-#define PIO_BF_MALLOC 0x0001 /* Buffer malloced */
-#define PIO_BF_MMAP 0x0002 /* Buffer mmap()ed */
-#define PIO_BF_LINEBUF 0x0004 /* Flushes on newline */
-#define PIO_BF_BLKBUF 0x0008 /* Raw block-based buffering */
+#define PIO_BF_MALLOC 0x0001 /* Buffer malloced */
+#define PIO_BF_MMAP 0x0002 /* Buffer mmap()ed */
+#define PIO_BF_LINEBUF 0x0004 /* Flushes on newline */
+#define PIO_BF_BLKBUF 0x0008 /* Raw block-based buffering */
+#define PIO_BF_UNDERFLOW 0x0010 /* Buffer failed to fill */
/* TODO: What is this? Figure it out and properly document it's use. */
#define PIO_NR_OPEN 256 /* Size of an "IO handle table" */
@@ -113,7 +114,6 @@ typedef struct _ParrotIOData ParrotIOData;
/* BUFFERING */
typedef struct _io_buffer {
INTVAL flags; /* Flags on this buffer */
- size_t raw_reads; /* Number of raw reads */
size_t buffer_size; /* Current allocated size */
const STR_VTABLE *encoding; /* Encoding used by this buffer */
char *buffer_ptr; /* ptr to the buffer mem block */
@@ -135,7 +135,6 @@ typedef INTVAL (*io_vtable_read_b) (PARROT_INTERP, PMC *handle, ARGOU
typedef INTVAL (*io_vtable_write_b) (PARROT_INTERP, PMC *handle, ARGIN(char * buffer), size_t byte_length);
typedef INTVAL (*io_vtable_flush) (PARROT_INTERP, PMC *handle);
typedef INTVAL (*io_vtable_is_eof) (PARROT_INTERP, PMC *handle);
-typedef void (*io_vtable_set_eof) (PARROT_INTERP, PMC *handle, INTVAL is_set);
typedef PIOOFF_T (*io_vtable_tell) (PARROT_INTERP, PMC *handle);
typedef PIOOFF_T (*io_vtable_seek) (PARROT_INTERP, PMC *handle, PIOOFF_T offset, INTVAL whence);
typedef void (*io_vtable_adv_position) (PARROT_INTERP, PMC *handle, size_t len);
@@ -158,7 +157,6 @@ typedef struct _io_vtable {
io_vtable_write_b write_b; /* Write bytes to the handle */
io_vtable_flush flush; /* Flush the handle */
io_vtable_is_eof is_eof; /* Determine if at end-of-file */
- io_vtable_set_eof set_eof; /* Set or clear the passed-EOF flag */
io_vtable_open open; /* Open the handle */
io_vtable_is_open is_open; /* Determine if the handle is open */
io_vtable_close close; /* Close the handle */
4 src/io/api.c
View
@@ -867,8 +867,6 @@ Parrot_io_read_byte_buffer_pmc(PARROT_INTERP, ARGMOD(PMC *handle),
account for whatever we read. */
if (bytes_read != byte_length)
VTABLE_set_integer_native(interp, buffer, bytes_read);
- if (bytes_read == 0)
- vtable->set_eof(interp, handle, 1);
vtable->adv_position(interp, handle, bytes_read);
return buffer;
}
@@ -962,7 +960,7 @@ Parrot_io_readline_s(PARROT_INTERP, ARGMOD(PMC *handle), ARGIN(STRING * terminat
io_verify_is_open_for(interp, handle, vtable, PIO_F_READ);
if (read_buffer == NULL)
- read_buffer = io_verify_has_read_buffer(interp, handle, vtable, BUFFER_SIZE_ANY);
+ read_buffer = io_verify_has_read_buffer(interp, handle, vtable, BUFFER_FLAGS_ANY);
/* Because of the way buffering works, the terminator sequence may be,
at most, one character shorter than half the size of the buffer.
26 src/io/buffer.c
View
@@ -117,8 +117,6 @@ Parrot_io_buffer_allocate(PARROT_INTERP, ARGMOD(PMC *owner), INTVAL flags,
buffer->buffer_end = buffer->buffer_ptr;
PARROT_ASSERT(BUFFER_IS_EMPTY(buffer));
- buffer->raw_reads = 0;
-
buffer->flags = flags;
return buffer;
}
@@ -293,8 +291,12 @@ Parrot_io_buffer_read_b(PARROT_INTERP, ARGMOD_NULLOK(IO_BUFFER *buffer),
size_t length)
{
ASSERT_ARGS(Parrot_io_buffer_read_b)
+ if(length == 0)
+ return 0;
+
if (!buffer)
return vtable->read_b(interp, handle, s, length);
+
{
size_t bytes_read = io_buffer_transfer_to_mem(interp, buffer, s, length);
PARROT_ASSERT(bytes_read <= length);
@@ -305,8 +307,11 @@ Parrot_io_buffer_read_b(PARROT_INTERP, ARGMOD_NULLOK(IO_BUFFER *buffer),
/* If we still need more data than the buffer can hold, just read it
directly. */
if (length > buffer->buffer_size) {
- bytes_read += vtable->read_b(interp, handle, s + bytes_read, length);
- buffer->raw_reads++;
+ const size_t count = vtable->read_b(interp, handle, s + bytes_read,
+ length);
+ bytes_read += count;
+ if (count == 0)
+ buffer->flags |= PIO_BF_UNDERFLOW;
}
/* Else, if we need to read an amount that the buffer can handle, fill
@@ -617,7 +622,9 @@ Parrot_io_buffer_fill(PARROT_INTERP, ARGMOD_NULLOK(IO_BUFFER *buffer),
return BUFFER_USED_SIZE(buffer);
read_bytes = vtable->read_b(interp, handle, buffer->buffer_end,
available_size);
- buffer->raw_reads++;
+ if (read_bytes == 0)
+ buffer->flags |= PIO_BF_UNDERFLOW;
+
buffer->buffer_end += read_bytes;
BUFFER_ASSERT_SANITY(buffer);
return BUFFER_USED_SIZE(buffer);
@@ -746,12 +753,9 @@ io_buffer_find_string_marker(PARROT_INTERP, ARGMOD(IO_BUFFER *buffer),
if (delim_bytelen == 1)
return bounds->bytes;
- /* If the buffer did not fill completely, we can assume there's nothing
- left for us to read because we tried to fill before we started this
- loop. If so, just return all the bytes in the buffer. If we've hit
- EOF and don't have the terminator, we'll never have it, so just
- return everything also. */
- if (BUFFER_FREE_END_SPACE(buffer) > 0 || vtable->is_eof(interp, handle))
+ /* If we've hit EOF and don't have the terminator, we'll never have it,
+ so just return all the bytes in the buffer. */
+ if (vtable->is_eof(interp, handle))
return bounds->bytes;
/* If the delimiter is multiple bytes, we might have part of it. We need
32 src/io/filehandle.c
View
@@ -108,13 +108,6 @@ static PIOOFF_T io_filehandle_seek(PARROT_INTERP,
__attribute__nonnull__(2)
FUNC_MODIFIES(*handle);
-static void io_filehandle_set_eof(PARROT_INTERP,
- ARGMOD(PMC *handle),
- INTVAL is_set)
- __attribute__nonnull__(1)
- __attribute__nonnull__(2)
- FUNC_MODIFIES(*handle);
-
static void io_filehandle_set_flags(PARROT_INTERP,
ARGIN(PMC *handle),
INTVAL flags)
@@ -185,9 +178,6 @@ static INTVAL io_filehandle_write_b(PARROT_INTERP,
#define ASSERT_ARGS_io_filehandle_seek __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
-#define ASSERT_ARGS_io_filehandle_set_eof __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
- PARROT_ASSERT_ARG(interp) \
- , PARROT_ASSERT_ARG(handle))
#define ASSERT_ARGS_io_filehandle_set_flags __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
@@ -235,7 +225,6 @@ io_filehandle_setup_vtable(PARROT_INTERP, ARGMOD_NULLOK(IO_VTABLE *vtable), INTV
vtable->write_b = io_filehandle_write_b;
vtable->flush = io_filehandle_flush;
vtable->is_eof = io_filehandle_is_eof;
- vtable->set_eof = io_filehandle_set_eof;
vtable->tell = io_filehandle_tell;
vtable->seek = io_filehandle_seek;
vtable->adv_position = io_filehandle_adv_position;
@@ -269,6 +258,12 @@ io_filehandle_read_b(PARROT_INTERP, ARGMOD(PMC *handle), ARGOUT(char *buffer), s
ASSERT_ARGS(io_filehandle_read_b)
const PIOHANDLE os_handle = io_filehandle_get_os_handle(interp, handle);
const size_t bytes_read = Parrot_io_internal_read(interp, os_handle, buffer, byte_length);
+ if (bytes_read == 0) {
+ INTVAL flags;
+ GETATTR_FileHandle_flags(interp, handle, flags);
+ flags |= PIO_F_EOF;
+ SETATTR_FileHandle_flags(interp, handle, flags);
+ }
return bytes_read;
}
@@ -317,11 +312,6 @@ io_filehandle_flush(PARROT_INTERP, ARGMOD(PMC *handle))
Determine if this handle as at end-of-file.
-=item C<static void io_filehandle_set_eof(PARROT_INTERP, PMC *handle, INTVAL
-is_set)>
-
-Set or clear the EOF flag.
-
=cut
*/
@@ -337,16 +327,6 @@ io_filehandle_is_eof(PARROT_INTERP, ARGMOD(PMC *handle))
return 0;
}
-static void
-io_filehandle_set_eof(PARROT_INTERP, ARGMOD(PMC *handle), INTVAL is_set)
-{
- ASSERT_ARGS(io_filehandle_set_eof)
- if (is_set)
- PARROT_FILEHANDLE(handle)->flags |= PIO_F_EOF;
- else
- PARROT_FILEHANDLE(handle)->flags &= ~PIO_F_EOF;
-}
-
/*
=item C<static PIOOFF_T io_filehandle_tell(PARROT_INTERP, PMC *handle)>
25 src/io/pipe.c
View
@@ -102,13 +102,6 @@ static PIOOFF_T io_pipe_seek(PARROT_INTERP,
__attribute__nonnull__(2)
FUNC_MODIFIES(*handle);
-static void io_pipe_set_eof(PARROT_INTERP,
- ARGMOD(PMC *handle),
- INTVAL is_set)
- __attribute__nonnull__(1)
- __attribute__nonnull__(2)
- FUNC_MODIFIES(*handle);
-
static void io_pipe_set_flags(PARROT_INTERP,
ARGIN(PMC *handle),
INTVAL flags)
@@ -179,9 +172,6 @@ static INTVAL io_pipe_write_b(PARROT_INTERP,
#define ASSERT_ARGS_io_pipe_seek __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
-#define ASSERT_ARGS_io_pipe_set_eof __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
- PARROT_ASSERT_ARG(interp) \
- , PARROT_ASSERT_ARG(handle))
#define ASSERT_ARGS_io_pipe_set_flags __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
@@ -226,7 +216,6 @@ io_pipe_setup_vtable(PARROT_INTERP, ARGMOD_NULLOK(IO_VTABLE *vtable), INTVAL idx
vtable->write_b = io_pipe_write_b;
vtable->flush = io_pipe_flush;
vtable->is_eof = io_pipe_is_eof;
- vtable->set_eof = io_pipe_set_eof;
vtable->tell = io_pipe_tell;
vtable->seek = io_pipe_seek;
vtable->adv_position = io_pipe_adv_position;
@@ -312,10 +301,6 @@ io_pipe_flush(PARROT_INTERP, ARGMOD(PMC *handle))
Determine if the pipe thinks it's at the end of input.
-=item C<static void io_pipe_set_eof(PARROT_INTERP, PMC *handle, INTVAL is_set)>
-
-Do nothing.
-
=cut
*/
@@ -331,16 +316,6 @@ io_pipe_is_eof(PARROT_INTERP, ARGMOD(PMC *handle))
return 0;
}
-static void
-io_pipe_set_eof(PARROT_INTERP, ARGMOD(PMC *handle), INTVAL is_set)
-{
- ASSERT_ARGS(io_pipe_set_eof)
- if (is_set)
- PARROT_FILEHANDLE(handle)->flags |= PIO_F_EOF;
- else
- PARROT_FILEHANDLE(handle)->flags &= ~PIO_F_EOF;
-}
-
/*
=item C<static PIOOFF_T io_pipe_tell(PARROT_INTERP, PMC *handle)>
34 src/io/socket.c
View
@@ -106,13 +106,6 @@ static PIOOFF_T io_socket_seek(PARROT_INTERP,
__attribute__nonnull__(2)
FUNC_MODIFIES(*handle);
-static void io_socket_set_eof(PARROT_INTERP,
- ARGMOD(PMC *handle),
- INTVAL is_eof)
- __attribute__nonnull__(1)
- __attribute__nonnull__(2)
- FUNC_MODIFIES(*handle);
-
static void io_socket_set_flags(PARROT_INTERP,
ARGIN(PMC *handle),
INTVAL flags)
@@ -183,9 +176,6 @@ static INTVAL io_socket_write_b(PARROT_INTERP,
#define ASSERT_ARGS_io_socket_seek __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
-#define ASSERT_ARGS_io_socket_set_eof __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
- PARROT_ASSERT_ARG(interp) \
- , PARROT_ASSERT_ARG(handle))
#define ASSERT_ARGS_io_socket_set_flags __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
@@ -230,7 +220,6 @@ io_socket_setup_vtable(PARROT_INTERP, ARGMOD_NULLOK(IO_VTABLE *vtable), INTVAL i
vtable->write_b = io_socket_write_b;
vtable->flush = io_socket_flush;
vtable->is_eof = io_socket_is_eof;
- vtable->set_eof = io_socket_set_eof;
vtable->tell = io_socket_tell;
vtable->seek = io_socket_seek;
vtable->adv_position = io_socket_adv_position;
@@ -308,12 +297,8 @@ io_socket_flush(PARROT_INTERP, ARGMOD(PMC *handle))
=item C<static INTVAL io_socket_is_eof(PARROT_INTERP, PMC *handle)>
-Sockets are not "passed-the-end" so long as the connection is open. Return 0.
-
-=item C<static void io_socket_set_eof(PARROT_INTERP, PMC *handle, INTVAL
-is_eof)>
-
-Do nothing.
+Sockets are considered at EOF if the last buffered read failed to fill the
+buffer. Always return 0 for unbuffered sockets.
=cut
@@ -323,18 +308,11 @@ static INTVAL
io_socket_is_eof(PARROT_INTERP, ARGMOD(PMC *handle))
{
ASSERT_ARGS(io_socket_is_eof)
- UNUSED(interp);
- UNUSED(handle);
- return 0;
-}
+ IO_BUFFER * const buffer = IO_GET_READ_BUFFER(interp, handle);
+ if (!buffer)
+ return 0;
-static void
-io_socket_set_eof(PARROT_INTERP, ARGMOD(PMC *handle), INTVAL is_eof)
-{
- ASSERT_ARGS(io_socket_set_eof)
- UNUSED(interp);
- UNUSED(handle);
- UNUSED(is_eof);
+ return !!(buffer->flags & PIO_BF_UNDERFLOW);
}
/*
25 src/io/stringhandle.c
View
@@ -106,13 +106,6 @@ static PIOOFF_T io_stringhandle_seek(PARROT_INTERP,
__attribute__nonnull__(2)
FUNC_MODIFIES(*handle);
-static void io_stringhandle_set_eof(PARROT_INTERP,
- ARGMOD(PMC *handle),
- INTVAL is_set)
- __attribute__nonnull__(1)
- __attribute__nonnull__(2)
- FUNC_MODIFIES(*handle);
-
static void io_stringhandle_set_flags(PARROT_INTERP,
ARGIN(PMC *handle),
INTVAL flags)
@@ -183,9 +176,6 @@ static INTVAL io_stringhandle_write_b(PARROT_INTERP,
#define ASSERT_ARGS_io_stringhandle_seek __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
-#define ASSERT_ARGS_io_stringhandle_set_eof __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
- PARROT_ASSERT_ARG(interp) \
- , PARROT_ASSERT_ARG(handle))
#define ASSERT_ARGS_io_stringhandle_set_flags __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
PARROT_ASSERT_ARG(interp) \
, PARROT_ASSERT_ARG(handle))
@@ -238,7 +228,6 @@ io_stringhandle_setup_vtable(PARROT_INTERP, ARGMOD_NULLOK(IO_VTABLE *vtable), IN
vtable->write_b = io_stringhandle_write_b;
vtable->flush = io_stringhandle_flush;
vtable->is_eof = io_stringhandle_is_eof;
- vtable->set_eof = io_stringhandle_set_eof;
vtable->tell = io_stringhandle_tell;
vtable->seek = io_stringhandle_seek;
vtable->adv_position = io_stringhandle_adv_position;
@@ -339,11 +328,6 @@ io_stringhandle_flush(PARROT_INTERP, ARGMOD(PMC *handle))
The StringHandle is at eof if the current read cursor is passed the end of the
string contents.
-=item C<static void io_stringhandle_set_eof(PARROT_INTERP, PMC *handle, INTVAL
-is_set)>
-
-Do nothing.
-
=cut
*/
@@ -359,15 +343,6 @@ io_stringhandle_is_eof(PARROT_INTERP, ARGMOD(PMC *handle))
return (UINTVAL)read_offs >= stringhandle->bufused;
}
-static void
-io_stringhandle_set_eof(PARROT_INTERP, ARGMOD(PMC *handle), INTVAL is_set)
-{
- ASSERT_ARGS(io_stringhandle_set_eof)
- UNUSED(interp);
- UNUSED(handle);
- UNUSED(is_set);
-}
-
/*
=item C<static PIOOFF_T io_stringhandle_tell(PARROT_INTERP, PMC *handle)>
80 src/io/utilities.c
View
@@ -278,7 +278,6 @@ io_read_encoded_string(PARROT_INTERP, ARGMOD(PMC *handle),
{
ASSERT_ARGS(io_read_encoded_string)
STRING * const s = Parrot_gc_new_string_header(interp, 0);
- const size_t raw_reads = buffer->raw_reads;
size_t total_bytes_read = 0;
Parrot_String_Bounds bounds;
size_t bytes_to_read = 0;
@@ -295,8 +294,8 @@ io_read_encoded_string(PARROT_INTERP, ARGMOD(PMC *handle),
/* When we have a request with PIO_READ_SIZE_ANY, we just want a big chunk
of data. Fill the buffer up as full as it gets and read it out. */
if (char_length == PIO_READ_SIZE_ANY) {
- /* FIXME: see io_readline_encoded_string() why checking against
- encoding->max_bytes_per_codepoint is a bad idea
+ /* FIXME: checking against encoding->max_bytes_per_codepoint is a
+ bad idea - we never might get that many bytes
*/
if (BUFFER_USED_SIZE(buffer) < encoding->max_bytes_per_codepoint)
Parrot_io_buffer_fill(interp, buffer, handle, vtable);
@@ -333,13 +332,11 @@ io_read_encoded_string(PARROT_INTERP, ARGMOD(PMC *handle),
/* Some types, like Socket, don't want to be read more than once in a
single request because recv can hang waiting for data. In those
cases, break out of the loop early. */
+/* FIXME - buffer-raw_reads is gone
if ((vtable->flags & PIO_VF_MULTI_READABLE) == 0 && buffer->raw_reads > raw_reads)
- break;
+ break; */
}
- if (total_bytes_read == 0)
- vtable->set_eof(interp, handle, 1);
-
return s;
}
@@ -397,8 +394,13 @@ io_read_chars_append_string(PARROT_INTERP, ARGMOD(STRING * s),
=item C<STRING * io_readline_encoded_string(PARROT_INTERP, PMC *handle, const
IO_VTABLE *vtable, IO_BUFFER *buffer, const STR_VTABLE *encoding, STRING * rs)>
-Read a line, up to and including the terminator character rs, from the
-input handle
+Read a line, up to and including the terminator string rs from the
+input handle.
+
+If the terminator string cannot be found, read until EOF is reached.
+
+This may block handles like sockets until the connection gets closed, which is
+necessary to ensure we never return incomplete lines.
=cut
@@ -413,12 +415,9 @@ io_readline_encoded_string(PARROT_INTERP, ARGMOD(PMC *handle),
{
ASSERT_ARGS(io_readline_encoded_string)
STRING * const s = Parrot_gc_new_string_header(interp, 0);
- size_t total_bytes_read = 0;
- const size_t raw_reads = buffer->raw_reads;
- size_t available_bytes = BUFFER_USED_SIZE(buffer);
- size_t prev_available_bytes = 0;
+
+ /* XXX: What do we do if encoding and rs->encoding don't agree? */
const size_t delim_size = STRING_byte_length(rs);
- INTVAL have_delim = 0;
s->bufused = 0;
s->strlen = 0;
@@ -428,55 +427,34 @@ io_readline_encoded_string(PARROT_INTERP, ARGMOD(PMC *handle),
s->encoding = encoding;
- while (!have_delim) {
+ while (1) {
Parrot_String_Bounds bounds;
- size_t bytes_to_read;
+ INTVAL have_delim = 0;
- /* We used to check against encoding->max_bytes_per_codepoint
- instead of encoding->bytes_per_unit.
+ /* Look for terminator and get amount of complete character data */
+ const size_t bytes_to_read = io_buffer_find_string_marker(
+ interp, buffer, handle, vtable, encoding, &bounds, rs, &have_delim);
- In case of variable-length codings like UTF-8, this meant waiting
- for more characters even if we already got all expected data and
- possibly hanging indefinitely if there's no more input available.
+ if (bytes_to_read != 0) {
+ /* Append buffer to result */
+ io_read_chars_append_string(interp, s, handle, vtable, buffer, bytes_to_read);
- FIXME: available_bytes < delim_size only makes sense if the
- encodings agree
- */
- if (available_bytes < delim_size || available_bytes < encoding->bytes_per_unit) {
- prev_available_bytes = available_bytes;
- available_bytes = Parrot_io_buffer_fill(interp, buffer, handle, vtable);
+ if (have_delim)
+ break;
}
- bytes_to_read = io_buffer_find_string_marker(interp, buffer, handle,
- vtable, encoding, &bounds, rs, &have_delim);
-
- /* Check if the buffer contains no (full) characters.
- If the last read returned nothing, assume we're at EOF and
- break out of the loop.
+ /* XXX: Should we discard any partial characters so vtable->is_eof()
+ and Parrot_io_eof() (and thus the PMC method) will agree?
- This used to check bytes_to_read == 0 only. In case of
- variable-length codings, the buffer may now contain a single
- partial character after a successful read, whereas it used to
- contain at least one full character.
+ Imo keeping the partial characters is the right thing to do.
*/
- if (bytes_to_read == 0 && prev_available_bytes == available_bytes)
+ if (vtable->is_eof(interp, handle))
break;
- /* Append buffer to result */
- io_read_chars_append_string(interp, s, handle, vtable, buffer, bytes_to_read);
- total_bytes_read += bytes_to_read;
- available_bytes -= bytes_to_read;
-
- /* Some types, like Socket, don't want to be read more than once in a
- single request because recv can hang waiting for data. In those
- cases, break out of the loop early. */
- if ((vtable->flags & PIO_VF_MULTI_READABLE) == 0 && buffer->raw_reads > raw_reads)
- break;
+ /* We haven't found the terminator yet, so get new input and try again */
+ Parrot_io_buffer_fill(interp, buffer, handle, vtable);
}
- if (total_bytes_read == 0)
- vtable->set_eof(interp, handle, 1);
-
return s;
}
56 src/pmc/filehandle.pmc
View
@@ -214,26 +214,6 @@ For internal usage only, subject to change without notice.
}
}
-
-/*
-
-=item C<INTVAL get_bool()>
-
-Return false if a previous read attempted to read past the end of the underlying
-filehandle. Note that this method may return true even if there are no bytes
-remaining if the most recent read requested the exact number of bytes remaining
-in the file.
-
-
-=cut
-
-*/
-
- VTABLE INTVAL get_bool() {
- return !Parrot_io_eof(INTERP, SELF);
- }
-
-
/*
=back
@@ -311,23 +291,6 @@ Returns a boolean value indicating whether C<SELF> is a console/tty.
RETURN(INTVAL isatty);
}
-
-/*
-
-=item C<METHOD is_closed()>
-
-Test if the filehandle is closed.
-
-=cut
-
-*/
-
- METHOD is_closed() {
- const INTVAL status = Parrot_io_is_closed(INTERP, SELF);
- RETURN(INTVAL status);
- }
-
-
/*
=item C<METHOD readline_interactive(STRING *prompt)>
@@ -553,25 +516,6 @@ Retrieve the read mode string for the filehandle.
/*
-=item C<METHOD eof()>
-
-Return true if a previous read attempted to read past the end of the underlying
-filehandle. Note that this method may return false even if there are no bytes
-remaining if the most recent read requested the exact number of bytes remaining
-in the file.
-
-=cut
-
-*/
-
- METHOD eof() {
- const INTVAL flags = Parrot_io_eof(INTERP, SELF);
- RETURN(INTVAL flags);
- }
-
-
-/*
-
=item C<METHOD handle()>
Returns the INTVAL used by the OS to identify this filehandle.
49 src/pmc/handle.pmc
View
@@ -94,6 +94,22 @@ pmclass Handle provides Handle manual_attrs {
/*
+=item C<INTVAL get_bool()>
+
+Returns whether the Handle is currently open.
+
+=cut
+
+*/
+
+ VTABLE INTVAL get_bool() {
+ const IO_VTABLE *io_vtable;
+ GET_ATTR_io_vtable(INTERP, SELF, io_vtable);
+ return io_vtable->is_open(INTERP, SELF);
+ }
+
+/*
+
=back
=head2 Methods
@@ -116,6 +132,39 @@ subtypes that are or can be tty.
/*
+=item C<METHOD eof()>
+
+Return true if a previous read attempted to read past the end of the underlying
+os handle. Note that this method may return false even if there are no bytes
+remaining if the most recent read requested the exact number of bytes remaining
+in the stream.
+
+=cut
+
+*/
+
+ METHOD eof() {
+ const INTVAL status = Parrot_io_eof(INTERP, SELF);
+ RETURN(INTVAL status);
+ }
+
+/*
+
+=item C<METHOD is_closed()>
+
+Test if the Handle is closed.
+
+=cut
+
+*/
+
+ METHOD is_closed() {
+ const INTVAL status = Parrot_io_is_closed(INTERP, SELF);
+ RETURN(INTVAL status);
+ }
+
+/*
+
=item C<METHOD get_fd()>
Retrieve the integer file descriptor for the Handle (only available on
31 src/pmc/socket.pmc
View
@@ -121,20 +121,6 @@ Free structures.
/*
-=item C<INTVAL get_bool()>
-
-Returns whether the Socket is currently open.
-
-=cut
-
-*/
-
- VTABLE INTVAL get_bool() {
- return !Parrot_io_is_closed(INTERP, SELF);
- }
-
-/*
-
=back
=head2 Methods
@@ -248,23 +234,6 @@ C<local_address> returns the local address of this socket PMC.
RETURN(PMC * res);
}
-
-/*
-
-=item C<METHOD is_closed()>
-
-Test if the socket is closed.
-
-=cut
-
-*/
-
- METHOD is_closed() {
- const INTVAL status = !VTABLE_get_bool(INTERP, SELF);
- RETURN(INTVAL status);
- }
-
-
/*
=item C<getprotobyname(STRING * name)>
55 src/pmc/stringhandle.pmc
View
@@ -143,27 +143,6 @@ Mark active stringhandle data as live.
/*
-=item C<INTVAL get_bool()>
-
-Returns whether the StringHandle has reached the end of the file.
-
-=cut
-
-*/
-
- VTABLE INTVAL get_bool() {
- STRING *stringhandle;
- GET_ATTR_stringhandle(INTERP, SELF, stringhandle);
-
- if (STRING_IS_NULL(stringhandle))
- return 0;
-
- return 1;
- }
-
-
-/*
-
=back
=head2 Methods
@@ -196,23 +175,6 @@ stored for mocking.
RETURN(PMC *handle);
}
-
-/*
-
-=item C<METHOD is_closed()>
-
-Check if the StringHandle is open.
-
-=cut
-
-*/
-
- METHOD is_closed() {
- const INTVAL closed = Parrot_io_is_closed(INTERP, SELF);
- RETURN(INTVAL closed);
- }
-
-
/*
=item METHOD readall(STRING *name);
@@ -366,23 +328,6 @@ Retrieve the read mode string for the stringhandle.
/*
-=item C<METHOD eof()>
-
-Check if the StringHandle is at end-of-file (if it has read to the end of the
-string data).
-
-=cut
-
-*/
-
- METHOD eof() {
- const INTVAL is_eof = Parrot_io_eof(INTERP, SELF);
- RETURN(INTVAL 0);
- }
-
-
-/*
-
=item C<METHOD get_fd()>
StringHandles do not use integer file descriptors, so always returns an error
2  t/pmc/filehandle.t
View
@@ -909,7 +909,7 @@ ok1:
ok2: say "ok 2"
$S0 = $P0.'read'(1024)
$S0 = $P0.'read'(1024)
- unless $P0, ok3
+ if $P0, ok3 # file at EOF is still true
print "not "
ok3: say "ok 3"
defined $I0, $P0
Something went wrong with that request. Please try again.