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] |
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?
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.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |