Re: Wide string strategies

[Ludovic Courtès]

Hi!

Thank you for working on this!

Mike Gran <address@hidden> writes:

> For now, I think a good strategy is to make strings into a pseudo-class
> where the internals are opaque to most of Guile and strings are accessed
> through accessors and other methods.  

Sounds reasonable.

> This was the strategy already begun with scm_to_locale_string but the
> code isn't fully committed to the idea.  The function scm_i_string_chars
> exposes the internal representation of the string, and it is used
> throughout the code.

Yes, that's going to be difficult, but I can't think of a better
solution.

> -  SCM_WTA_DISPATCH_1 (*SCM_SUBR_GENERIC (proc), arg1,
> -                   SCM_ARG1, scm_i_symbol_chars (SCM_SNAME (proc)));
> +  {
> +    char *str = scm_to_locale_string (scm_symbol_to_string (SCM_SNAME 
> (proc)));
> +    SCM_WTA_DISPATCH_1 (*SCM_SUBR_GENERIC (proc), arg1, SCM_ARG1, str);
> +    free (str);
> +  }

This is the kind of thing we can't afford in most cases.

Here STR is only needed because `SCM_WTA_DISPATCH_1 ()' calls
`scm_wrong_type_arg ()', which operates on C strings.

One solution would be to change `scm_wrong_type_arg ()' to operate on
opaque strings (e.g., take an `SCM' instead of `const char *').  The
same applies to all the functions in "error.h", and probably many
others.

> -      if (len > 0 && (s[0] == '/' || s[0] == '\\'))
> +      if (len > 0 && (scm_i_string_ref_eq_char (filename, 0, '/') 
> +                   || scm_i_string_ref_eq_char (filename, 0, '\\')))

I think procedures like `scm_i_string_ref_eq_char ()' are a good idea
because it fulfills the goal of having an opaque string type *and* the
goal of being able to handle them easily in C.

> -  scm_ungets (scm_i_string_chars (str), scm_i_string_length (str), port);
> +  buf = scm_to_locale_stringn (str, &len);
> +  scm_ungets (buf, len, port);
> +  free (buf);

Eventually, we might need to change `scm_ungets ()', or provide a
variant that takes an opaque string.

> @@ -1549,12 +1552,16 @@ SCM_DEFINE (scm_recvfrom, "recvfrom!", 2, 3, 0,
>  
>    /* recvfrom will not necessarily return an address.  usually nothing
>       is returned for stream sockets.  */
> -  buf = scm_i_string_writable_chars (str);
> +  buf = scm_malloc (cend - offset);

All the POSIX interface needs fast access to ASCII strings.  How about
something like:

  const char *layout = scm_i_ascii_symbol_chars (SCM_PACK (slayout));

where `scm_i_ascii_symbol_chars ()' throws an exception if its argument
is a non-ASCII symbol?

This would mean special-casing ASCII stringbufs so that we can treat
them as C strings.

> +static const char *
> +make_failsafe_ascii_sz (const char *str, size_t len)
> +{
> +  static char buf[SCM_FAILSAFE_STRING_LEN];

Ouch, that would be bug-prone.

> +SCM_INTERNAL int scm_i_string_ref_eq_char (SCM str, size_t x, char c);
> +SCM_INTERNAL int scm_i_symbol_ref_eq_char (SCM str, size_t x, char c);
> +SCM_INTERNAL int scm_i_string_ref_neq_char (SCM str, size_t x, char c);
> +SCM_INTERNAL int scm_i_symbol_ref_neq_char (SCM str, size_t x, char c);

I'd remove the `neq' variants.

> +SCM_INTERNAL int scm_i_string_ref_eq_int (SCM str, size_t x, int c);

Does it assume sizeof (int) >= 32 ?

> +SCM_INTERNAL size_t scm_i_string_contains_char (SCM str, char ch);

Since it really returns a boolean, I'd use `int' as the return type.

> +SCM_INTERNAL char *scm_i_string_to_write_sz (SCM str);
> +SCM_INTERNAL scm_t_uint8 *scm_i_string_to_u8sz (SCM str);
> +SCM_INTERNAL SCM scm_i_string_from_u8sz (const scm_t_uint8 *str);
> +SCM_INTERNAL const char *scm_i_string_to_failsafe_ascii_sz (SCM str);
> +SCM_INTERNAL const char *scm_i_symbol_to_failsafe_ascii_sz (SCM str);

What does "sz" mean?

> +/* For ASCII strings, SUB can be used to represent an invalid
> +   character.  */
> +#define SCM_SUB ('\x1A')

Why SUB?  How about `SCM_I_SUB_CHAR', `SCM_I_INVALID_ASCII_CHAR' or
similar?

Thanks,
Ludo'.