This is the mail archive of the archer@sourceware.org mailing list for the Archer project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 06/11/2009 06:11 PM, Tom Tromey wrote:


Hi Tom


Thanks for the comments. Most are done, but I have some further questions with others.


Phil> + int is_string = 0;

Phil>  -	  if (gdbpy_is_string (result))
Phil>  -	    output = python_string_to_host_string (result);
Phil>  -	  else if (PyObject_TypeCheck (result,&value_object_type))
Phil>  +	  is_string = gdbpy_is_string (result);
Phil>  +	  if (! is_string)

I think the previous code was clearer. Don't introduce is_string,
instead...


Agreed, fixed.


Phil> + Py_DECREF (result);

Set 'result = NULL' here.


Likewise


Phil> +unicode_to_encoded_string (PyObject *unicode_str, const char *charset, int *size)

[...]

Phil>  +  str_size = PyString_Size (string) + 1;
Phil>  +  result = xmemdup (PyString_AsString (string), str_size, str_size);
Phil>  +  if (size != NULL)
Phil>  +    *size = str_size;

How about a variant of this function that just returns the
intermediate 'string' PyObject?

To avoid the use of an out-parameter? I can see the use of returning an encoded version of the the unicode PyObject string, but if you wanted that, why would you just not call:


PyUnicode_AsEncodedString directly? Wouldn't the following:

static PyObject*
unicode_to_encoded_string (PyObject *unicode_str, const char *charset)
{
return PyUnicode_AsEncodedString (unicode_str, charset, NULL);
}

Be the same equivalent? What would the purpose of a new variant achieve there?


Phil>  +      output = python_string_to_target_string (py_str,&len);
Phil>  +      if (output)
Phil>  +	{
Phil>  +	  struct cleanup *old = make_cleanup (xfree, output);

Adding a new unicode_to_target_string variant (or
python_string_to_target_string, or whatever is nicest) means you can
avoid an allocation here.


Similar to above, I'm not sure what you mean here. python_string_to_target_string does more than than the previous hunk comment, but eventually will have to be converted to char * to pass through the LA_PRINT_STRING that is directly below this code?



Phil>  --- a/gdb/varobj.c
Phil>  +	    Py_DECREF(py_str);

Space before open paren.


Fixed thanks!


Regards

Phil


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]