Re: [python][patch] Preserve string length, and embedded-nulls in pretty-print string emission.

[Tom Tromey <tromey at redhat dot com>]

Phil> +convert_value_from_python (PyObject *obj, int *size)

I don't understand why this needs the additional argument.
It seems to me that the size is implicit in the returned value.

Phil> +	  if (PyErr_Occurred ())
Phil> +	      *out_value = NULL;

This looks like it is indented improperly.

Phil> +  str_length = pretty_print_one_value (printer, &replacement);
Phil> +  if (replacement)
Phil> +    if (str_length > 0)
Phil> +      {
Phil> +	LA_GET_STRING (replacement, &output, &str_length, &la_encoding);
Phil> +	if (hint && !strcmp (hint, "string"))
Phil> +	  LA_PRINT_STRING (stream, output, str_length, 1, 0, options);
Phil> +	else
Phil> +	  fputs_filtered (output, stream);
Phil> +	xfree (output);

Why do we need LA_GET_STRING here?  Aren't the string contents already
in the value?

There are a couple cases like this.

Also, why check "str_length > 0".  A string might have length 0.
I think instead you probably want some kind of type check.

Phil>    *replacement = NULL;
Phil> -  result = pretty_print_one_value (printer_obj, replacement);
Phil> -  if (result == NULL);
Phil> +  size = pretty_print_one_value (printer_obj, replacement);
Phil> +
Phil> +  if (replacement == NULL);
Phil>      gdbpy_print_stack ();

This should check *replacement; 'replacement' can never be NULL.

Phil> +std::string cplus_str ("embedded\0null\0string",20);

A test relying on std::string is not very robust -- std::string may
change over time, breaking the test.  Instead, it is better to write
custom test code and a printer to match.  That way, we control the
implementation.

Tom