Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

We’re showing branches in this repository, but you can also compare across forks.

base: master
...
compare: tied-cstring
  • 8 commits
  • 9 files changed
  • 4 commit comments
  • 1 contributor
1  include/parrot/pobj.h
View
@@ -93,6 +93,7 @@ struct parrot_string_t {
/* parrot_string_representation_t representation;*/
const struct _str_vtable *encoding; /* Pointer to string vtable. */
+ char *tied_cstr; /* cstring tied to STRING's lifetime */
};
/* Here is the Parrot PMC object, "inheriting" from PObj. */
3  src/gc/gc_gms.c
View
@@ -1531,6 +1531,9 @@ gc_gms_free_string_header(PARROT_INTERP, ARGFREE(STRING *s))
Parrot_pa_remove(interp, self->strings[gen], STR2PAC(s)->ptr);
+ if (s->tied_cstr)
+ Parrot_str_free_cstring(s->tied_cstr);
+
if (Buffer_bufstart(s) && !PObj_external_TEST(s))
Parrot_gc_str_free_buffer_storage(interp,
&self->string_gc, (Buffer *)s);
2  src/gc/gc_inf.c
View
@@ -290,6 +290,8 @@ static void
gc_inf_free_string_header(SHIM_INTERP, ARGFREE(STRING *s))
{
ASSERT_ARGS(gc_inf_free_string_header)
+ if (s->tied_cstr)
+ Parrot_str_free_cstring(s->tied_cstr);
if (s)
free(s);
}
2  src/gc/gc_ms.c
View
@@ -953,6 +953,8 @@ gc_ms_free_string_header(PARROT_INTERP, ARGMOD(STRING *s))
Memory_Pools * const mem_pools = (Memory_Pools *)interp->gc_sys->gc_private;
if (!PObj_constant_TEST(s)) {
Fixed_Size_Pool * const pool = mem_pools->string_header_pool;
+ if (s->tied_cstr)
+ Parrot_str_free_cstring(s->tied_cstr);
PObj_flags_SETTO((PObj *)s, PObj_on_free_list_FLAG);
pool->add_free_object(interp, mem_pools, pool, s);
++pool->num_free_objects;
3  src/gc/gc_ms2.c
View
@@ -832,6 +832,9 @@ gc_ms2_free_string_header(PARROT_INTERP, ARGFREE(STRING *s))
Parrot_pa_remove(interp, self->strings, STR2PAC(s)->ptr);
+ if (s->tied_cstr)
+ Parrot_str_free_cstring(s->tied_cstr);
+
if (Buffer_bufstart(s) && !PObj_external_TEST(s))
Parrot_gc_str_free_buffer_storage(interp,
&self->string_gc, (Buffer *)s);
8 src/nci/libffi.c
View
@@ -339,6 +339,7 @@ nci_to_ffi_type(PARROT_INTERP, PARROT_DATA_TYPE nci_t)
case enum_type_STRING:
case enum_type_ptr:
+ case enum_type_cstr:
case enum_type_PMC:
return &ffi_type_pointer;
@@ -619,6 +620,13 @@ call_ffi_thunk(PARROT_INTERP, ARGMOD(PMC *nci_pmc), ARGMOD(PMC *self))
VTABLE_get_pointer(interp, pcc_arg[i].p);
nci_arg_ptr[i] = &nci_val[i].p;
break;
+ case enum_type_cstr:
+ nci_val[i].p = STRING_IS_NULL(pcc_arg[i].s) ?
+ "":
+ Parrot_str_to_tied_cstring(interp, pcc_arg[i].s);
+ nci_arg_ptr[i] = &nci_val[i].p;
+ break;
+
default:
PARROT_ASSERT("Unhandled NCI signature");
4 src/nci/signatures.c
View
@@ -99,6 +99,9 @@ Parrot_nci_parse_signature(PARROT_INTERP, ARGIN(STRING *sig_str))
case 'S':
e = enum_type_STRING;
break;
+ case 't':
+ e = enum_type_cstr;
+ break;
case 'p': /* push pmc->data */
e = enum_type_ptr;
@@ -157,6 +160,7 @@ ncidt_to_pcc(PARROT_INTERP, PARROT_DATA_TYPE t)
case enum_type_ptr:
case enum_type_PMC:
+ case enum_type_cstr:
return 'P';
default:
30 src/string/api.c
View
@@ -218,6 +218,7 @@ Parrot_str_new_noinit(PARROT_INTERP, UINTVAL capacity)
Parrot_gc_allocate_string_storage(interp, s,
(size_t)string_max_bytes(interp, s, capacity));
+ s->tied_cstr = NULL;
return s;
}
@@ -358,6 +359,7 @@ Parrot_str_clone(PARROT_INTERP, ARGIN_NULLOK(const STRING *s))
result->strlen = s->strlen;
result->hashval = s->hashval;
result->encoding = s->encoding;
+ result->tied_cstr = NULL;
return result;
}
@@ -682,6 +684,7 @@ Parrot_str_new_init(PARROT_INTERP, ARGIN_NULLOK(const char *buffer), UINTVAL len
}
else
s->strlen = s->bufused = 0;
+ s->tied_cstr = NULL;
return s;
}
@@ -1245,6 +1248,7 @@ Parrot_str_replace(PARROT_INTERP, ARGIN(const STRING *src),
/* Alloctate new string size. */
Parrot_gc_allocate_string_storage(interp, dest, buf_size);
dest->bufused = buf_size;
+ dest->tied_cstr = NULL;
/* Copy begin of string */
mem_sys_memcopy(dest->strstart, src->strstart, start_byte);
@@ -2113,6 +2117,31 @@ Parrot_str_from_num(PARROT_INTERP, FLOATVAL f)
/*
+=item C<char * Parrot_str_to_tied_cstring(PARROT_INTERP, const STRING *s)>
+
+Returns a C string for the specified Parrot string in the current
+representation of the string. The C string's lifetime is tied to the STRING*.
+It should not be manually freed.
+
+=cut
+
+*/
+
+PARROT_EXPORT
+PARROT_CANNOT_RETURN_NULL
+const char *
+Parrot_str_to_tied_cstring(PARROT_INTERP, ARGIN(STRING *s))
+{
+ ASSERT_ARGS(Parrot_str_to_cstring)
+
+ if (!s->tied_cstr)
+ s->tied_cstr = Parrot_str_to_encoded_cstring(interp, s, s->encoding);
+ return s->tied_cstr;
+}
+
+
+/*
+
=item C<char * Parrot_str_to_cstring(PARROT_INTERP, const STRING *s)>
Returns a C string for the specified Parrot string in the current
@@ -2562,6 +2591,7 @@ Parrot_str_unescape_string(PARROT_INTERP, ARGIN(const STRING *src),
reserved = string_max_bytes(interp, result, srclen);
Parrot_gc_allocate_string_storage(interp, result, reserved);
result->bufused = reserved;
+ result->tied_cstr = NULL;
STRING_ITER_INIT(interp, &itersrc);
STRING_ITER_INIT(interp, &iterdest);
11 tools/dev/nci_thunk_gen.pir
View
@@ -877,7 +877,7 @@ ERROR
(empty_line_regex, pcre_errstr, pcre_errint) = pcre_comp($S0, pcre_extended)
if pcre_errint goto pcre_comp_err
- $S0 = "^ [[:space:]]* ( (?: [INSPcsilfdpv] [[:space:]]* )+ ) (?: [#] .* )? $"
+ $S0 = "^ [[:space:]]* ( (?: [INSPcsilfdptv] [[:space:]]* )+ ) (?: [#] .* )? $"
(old_style_sig_line_regex, pcre_errstr, pcre_errint) = pcre_comp($S0, pcre_extended)
if pcre_errint goto pcre_comp_err
@@ -987,6 +987,15 @@ REGEX
JSON
table[.DATATYPE_PTR] = $P1
+ $P1 = 'from_json'(<<'JSON')
+{ "c_type": "char *",
+ "pcc_type": "STRING *",
+ "preamble_tmpl": "v_%i = STRING_IS_NULL(t_%i) ? '' : Parrot_str_get_tied_cstring(interp, t_%i);",
+ "sig_char": "t"
+ }
+JSON
+ table[.DATATYPE_CSTR] = $P1
+
$P1 = 'from_json'('{ "c_type": "char", "sig_char": "I", "pcc_type": "INTVAL" }')
table[.DATATYPE_CHAR] = $P1

Showing you all comments on commits in this comparison.

chromatic

This will be the source of countless bugs in the future. The memory model of any arbitrary shared library is almost certainly different from Parrot's memory model. A shared library may:

  • read from the string
  • write to the string
  • store the string and free it itself
  • store the string and expect a separate call internal to the shared library to free it
  • store the string and expect the calling code to free it
  • expect the string to confirm to specific encoding characteristics

Expecting a single 't' type for NULL-terminated C strings to provide any or all of these semantics simultaneously is a recipe for data corruption and segfaults.

Vasily Chekalkin
Collaborator
Vasily Chekalkin
Collaborator
cotto
Owner

Yes. I was working on this as a quick solution to some nci troubles we were having, but even then I wouldn't let it go into master without a very clear warning and explanation about what it does and doesn't do.

Something went wrong with that request. Please try again.