This is the mail archive of the 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.

Tom Tromey wrote:
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.

This function returns a struct value. If that value contains a string with embedded nulls, is there a way to determine the size of the "gdb_byte *contents"? I might have missed a method - or the obvious - but this is the reason for the out parameter. If the parameter size is not NULL, the size of the string is returned there.

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

This looks like it is indented improperly.

Ok, thanks.

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?

I'll look at this a little more closely. But I use this to extract a string with embedded nulls with a concrete length. Do I misread your thoughts here? Or are you suggesting we just pass in value->contents to LA_PRINT_STRING?

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

Ok, thanks.

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.

Ok, thanks.

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

Ok thanks for the pointer. Can you please elaborate a bit more?



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