Verify that the record_separator is the right encoding, even if it is…

… set after the encoding is.

Factor out some re-encoding logic into a new helper function. fix the .record_separator() method to accept strings of sizes other than 1 character

include/parrot/io.h

@@ -645,6 +645,15 @@ void Parrot_io_mark(PARROT_INTERP, ARGIN(ParrotIOData *piodata))
         __attribute__nonnull__(1)
         __attribute__nonnull__(2);
 
+PARROT_CANNOT_RETURN_NULL
+PARROT_WARN_UNUSED_RESULT
+STRING * Parrot_io_reencode_string_for_handle(PARROT_INTERP,
+    ARGIN(PMC *handle),
+    ARGIN(STRING *str))
+        __attribute__nonnull__(1)
+        __attribute__nonnull__(2)
+        __attribute__nonnull__(3);
+
 void Parrot_io_set_buffer_mode(PARROT_INTERP,
     ARGMOD(PMC *handle),
     ARGIN(STRING *mode))
@@ -841,6 +850,11 @@ INTVAL Parrot_io_write_byte_buffer_pmc(PARROT_INTERP,
 #define ASSERT_ARGS_Parrot_io_mark __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
        PARROT_ASSERT_ARG(interp) \
     , PARROT_ASSERT_ARG(piodata))
+#define ASSERT_ARGS_Parrot_io_reencode_string_for_handle \
+     __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
+       PARROT_ASSERT_ARG(interp) \
+    , PARROT_ASSERT_ARG(handle) \
+    , PARROT_ASSERT_ARG(str))
 #define ASSERT_ARGS_Parrot_io_set_buffer_mode __attribute__unused__ int _ASSERT_ARGS_CHECK = (\
        PARROT_ASSERT_ARG(interp) \
     , PARROT_ASSERT_ARG(handle) \

src/io/api.c

@@ -2037,6 +2037,42 @@ Parrot_io_get_buffer_mode(PARROT_INTERP, ARGMOD(PMC *handle))
 
 /*
 
+=item C<STRING * Parrot_io_reencode_string_for_handle(PARROT_INTERP, PMC
+*handle, STRING *str)>
+
+Verify that the given string matches the encoding for the given handle. If so,
+return it directly. If not, create a new string with the proper encoding and
+return that.
+
+=cut
+
+*/
+
+PARROT_CANNOT_RETURN_NULL
+PARROT_WARN_UNUSED_RESULT
+STRING *
+Parrot_io_reencode_string_for_handle(PARROT_INTERP, ARGIN(PMC *handle), ARGIN(STRING *str))
+{
+    ASSERT_ARGS(Parrot_io_reencode_string_for_handle)
+
+    if (PMC_IS_NULL(handle))
+        Parrot_ex_throw_from_c_args(interp, NULL, EXCEPTION_PIO_ERROR,
+            "Handle may not be null");
+    if (STRING_IS_NULL(str))
+        return STRINGNULL;
+
+    {
+        const IO_VTABLE * const vtable = IO_GET_VTABLE(interp, handle);
+        const STR_VTABLE * const encoding = vtable->get_encoding(interp, handle);
+
+        if (encoding != NULL && str->encoding != encoding)
+            return encoding->to_encoding(interp, str);
+        return str;
+    }
+}
+
+/*
+
 =back
 
 =cut

src/pmc/handle.pmc

@@ -201,10 +201,11 @@ Set the record separator for readline.
 
     METHOD record_separator(STRING *str :optional, int has_str :opt_flag) {
         if (has_str) {
-            if (STRING_length(str) != 1)
+            if (STRING_IS_NULL(str))
                 Parrot_ex_throw_from_c_args(INTERP, NULL, EXCEPTION_INVALID_OPERATION,
-                    "Record separator must be a single character");
+                    "Record separator may not be null");
 
+            str = Parrot_io_reencode_string_for_handle(INTERP, SELF, str);
             SET_ATTR_record_separator(INTERP, SELF, str);
         }
         else {
@@ -241,34 +242,18 @@ scheme) for the filehandle.
 
 */
 
-    METHOD encoding(STRING *new_encoding :optional, INTVAL got_encoding :opt_flag) {
-        STRING *encoding;
-
+    METHOD encoding(STRING *encoding :optional, INTVAL got_encoding :opt_flag) {
         if (got_encoding) {
-            /* TODO: This is something of a hack. We need to make sure the
-               record terminator has the right encoding for when we do
-               .readline(), but we don't want to re-encode it for every single
-               call to .readline(). We'll re-encode it here, since .encoding()
-               calls are less common than .readline() calls. */
-
             STRING * record_separator;
-            STR_VTABLE * encoding;
-            IO_VTABLE * vtable;
 
-            SET_ATTR_encoding(INTERP, SELF, new_encoding);
+            SET_ATTR_encoding(INTERP, SELF, encoding);
             GET_ATTR_record_separator(INTERP, SELF, record_separator);
-            GET_ATTR_io_vtable(INTERP, SELF, vtable);
-            encoding = vtable->get_encoding(INTERP, SELF);
-
-            if (encoding != NULL && record_separator->encoding != encoding) {
-                record_separator = encoding->to_encoding(INTERP, record_separator);
-                SET_ATTR_record_separator(INTERP, SELF, record_separator);
-            }
-
-            RETURN(STRING *new_encoding);
+            record_separator = Parrot_io_reencode_string_for_handle(INTERP, SELF, record_separator);
+            SET_ATTR_record_separator(INTERP, SELF, record_separator);
+        }
+        else {
+            GET_ATTR_encoding(INTERP, SELF, encoding);
         }
-
-        GET_ATTR_encoding(INTERP, SELF, encoding);
         RETURN(STRING *encoding);
     }
 }