From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 014753858D38 for ; Mon, 11 Sep 2023 14:18:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 014753858D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694441937; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Kkhet8ZZbntgQRSb+GHZOLvWWYv89gChB5jRNl2okLI=; b=jCbZqmTJy0bOZCZsdaMVXdDkSVxbi4VE7uT5ZhvYR11xId29vJBbh5PuWBvjiU1jN6Nr1J OFZ2LI/J74vn1Y5SfThibjZKyKrI6KL/nNdDFBNgQHvMUx0uar6i7gH/4fI8HI4yGG1GRU TZuMZvhtnMKfUvrocaXjGtdgVULFtT8= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-407-2dUkzpkpPsaYN7s3RVw4EQ-1; Mon, 11 Sep 2023 10:18:56 -0400 X-MC-Unique: 2dUkzpkpPsaYN7s3RVw4EQ-1 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-402493d2997so32253735e9.0 for ; Mon, 11 Sep 2023 07:18:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694441934; x=1695046734; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Kkhet8ZZbntgQRSb+GHZOLvWWYv89gChB5jRNl2okLI=; b=sKeiR1y/NN44olpaR9Ak0mpNeEZbkx2YicSolzrBbjOkIMkBqgdIZ1qvOVCCey9TMI PybufzVztginCKFXtrioHUWHa2wtKSXNLLN6nB/DedFl07E0yyMrRYaUOD9aTsiwhnn6 7VCr+bHr5p3KE1puheWs2sRqeORWNNt+QqEAq7AUxsHMVDIU6tlXeiWzEg8puZkQMaCs UktDnB2jY8hbV/Js8dDVwmnguR/UZeL4cXuFaVb2qYLEER298v7EfwfHF6Sh1A7jk8Jg 0PclodN7pKTeh4UpNUzCe7kUSSKtn97g64iTBRM0YFNbc4p2OcAFdU1QD97tjHECCAFq oqcA== X-Gm-Message-State: AOJu0Yz3sWTqHBDv5jN/Tf6behWfYr/K+cO25V960oPsI+KHWbIl8FdY g5Or6BQQ0Pq7bs7F9w8OHNO4BTlxTAhp6nRYZmu4ifRfX6vxwWEbeuPiX6bk8hrqUObcsIkN1nD 4wRpT/9B/5SWPhMt4hISCVXfjNHc9o2HXPo4Ca6Pe/+ZWXVm3bPHi2rAYpeSX82XPuvsgitf8PR WNoqgw2Q== X-Received: by 2002:a05:600c:290a:b0:402:e6a2:c8c7 with SMTP id i10-20020a05600c290a00b00402e6a2c8c7mr8910174wmd.7.1694441934668; Mon, 11 Sep 2023 07:18:54 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGXCRQ94gGudNY+mvw5ovVHH8Issvzb+HtrlRWJemop5LvPmKlo9AuG3pvBO4pm5IP87LpJjg== X-Received: by 2002:a05:600c:290a:b0:402:e6a2:c8c7 with SMTP id i10-20020a05600c290a00b00402e6a2c8c7mr8910149wmd.7.1694441934112; Mon, 11 Sep 2023 07:18:54 -0700 (PDT) Received: from localhost (197.126.90.146.dyn.plus.net. [146.90.126.197]) by smtp.gmail.com with ESMTPSA id z21-20020a05600c221500b003fbb25da65bsm10210367wml.30.2023.09.11.07.18.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 Sep 2023 07:18:53 -0700 (PDT) From: Andrew Burgess To: Jan Vrany via Gdb-patches , gdb-patches@sourceware.org Cc: Jan Vrany Subject: Re: [PATCH 1/2] gdb/python: generalize serialize_mi_result() In-Reply-To: <20230908210504.89194-1-jan.vrany@labware.com> References: <20230908210504.89194-1-jan.vrany@labware.com> Date: Mon, 11 Sep 2023 15:18:52 +0100 Message-ID: <874jk0lrn7.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Jan Vrany via Gdb-patches writes: > This commit renames serialize_mi_result() to serialize_mi_data() and makes > it usable in different contexts than printing result of custom MI command > (hence the rename). > > To do so, the check whether passed Python object is a dictionary has been > moved to the caller - at the very least, different usages require different > error messages. Also it seemed to me that py-utils.c is a better place > for its code as it is now a utility function not specific to Python MI > commands. It's no longer just for MI commands, but it probably is only for MI "stuff". I wonder if moving it to py-mi.c would be better? > > This is a preparation for implementing Python support for sending custom > MI async events. > --- > gdb/python/py-micmd.c | 179 ++--------------------------------- > gdb/python/py-utils.c | 145 ++++++++++++++++++++++++++++ > gdb/python/python-internal.h | 15 +++ > 3 files changed, 166 insertions(+), 173 deletions(-) > > diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c > index 01fc6060ece..26231fc37e8 100644 > --- a/gdb/python/py-micmd.c > +++ b/gdb/python/py-micmd.c > @@ -173,178 +173,6 @@ extern PyTypeObject micmdpy_object_type > > static PyObject *invoke_cst; > > -/* Convert KEY_OBJ into a string that can be used as a field name in MI > - output. KEY_OBJ must be a Python string object, and must only contain > - characters suitable for use as an MI field name. > - > - If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters, > - then an error is thrown. Otherwise, KEY_OBJ is converted to a string > - and returned. */ > - > -static gdb::unique_xmalloc_ptr > -py_object_to_mi_key (PyObject *key_obj) > -{ > - /* The key must be a string. */ > - if (!PyUnicode_Check (key_obj)) > - { > - gdbpy_ref<> key_repr (PyObject_Repr (key_obj)); > - gdb::unique_xmalloc_ptr key_repr_string; > - if (key_repr != nullptr) > - key_repr_string = python_string_to_target_string (key_repr.get ()); > - if (key_repr_string == nullptr) > - gdbpy_handle_exception (); > - > - gdbpy_error (_("non-string object used as key: %s"), > - key_repr_string.get ()); > - } > - > - gdb::unique_xmalloc_ptr key_string > - = python_string_to_target_string (key_obj); > - if (key_string == nullptr) > - gdbpy_handle_exception (); > - > - /* Predicate function, returns true if NAME is a valid field name for use > - in MI result output, otherwise, returns false. */ > - auto is_valid_key_name = [] (const char *name) -> bool > - { > - gdb_assert (name != nullptr); > - > - if (*name == '\0' || !isalpha (*name)) > - return false; > - > - for (; *name != '\0'; ++name) > - if (!isalnum (*name) && *name != '_' && *name != '-') > - return false; > - > - return true; > - }; > - > - if (!is_valid_key_name (key_string.get ())) > - { > - if (*key_string.get () == '\0') > - gdbpy_error (_("Invalid empty key in MI result")); > - else > - gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ()); > - } > - > - return key_string; > -} > - > -/* Serialize RESULT and print it in MI format to the current_uiout. > - FIELD_NAME is used as the name of this result field. > - > - RESULT can be a dictionary, a sequence, an iterator, or an object that > - can be converted to a string, these are converted to the matching MI > - output format (dictionaries as tuples, sequences and iterators as lists, > - and strings as named fields). > - > - If anything goes wrong while formatting the output then an error is > - thrown. > - > - This function is the recursive inner core of serialize_mi_result, and > - should only be called from that function. */ > - > -static void > -serialize_mi_result_1 (PyObject *result, const char *field_name) > -{ > - struct ui_out *uiout = current_uiout; > - > - if (PyDict_Check (result)) > - { > - PyObject *key, *value; > - Py_ssize_t pos = 0; > - ui_out_emit_tuple tuple_emitter (uiout, field_name); > - while (PyDict_Next (result, &pos, &key, &value)) > - { > - gdb::unique_xmalloc_ptr key_string > - (py_object_to_mi_key (key)); > - serialize_mi_result_1 (value, key_string.get ()); > - } > - } > - else if (PySequence_Check (result) && !PyUnicode_Check (result)) > - { > - ui_out_emit_list list_emitter (uiout, field_name); > - Py_ssize_t len = PySequence_Size (result); > - if (len == -1) > - gdbpy_handle_exception (); > - for (Py_ssize_t i = 0; i < len; ++i) > - { > - gdbpy_ref<> item (PySequence_ITEM (result, i)); > - if (item == nullptr) > - gdbpy_handle_exception (); > - serialize_mi_result_1 (item.get (), nullptr); > - } > - } > - else if (PyIter_Check (result)) > - { > - gdbpy_ref<> item; > - ui_out_emit_list list_emitter (uiout, field_name); > - while (true) > - { > - item.reset (PyIter_Next (result)); > - if (item == nullptr) > - { > - if (PyErr_Occurred () != nullptr) > - gdbpy_handle_exception (); > - break; > - } > - serialize_mi_result_1 (item.get (), nullptr); > - } > - } > - else > - { > - if (PyLong_Check (result)) > - { > - int overflow = 0; > - gdb_py_longest val = gdb_py_long_as_long_and_overflow (result, > - &overflow); > - if (PyErr_Occurred () != nullptr) > - gdbpy_handle_exception (); > - if (overflow == 0) > - { > - uiout->field_signed (field_name, val); > - return; > - } > - /* Fall through to the string case on overflow. */ > - } > - > - gdb::unique_xmalloc_ptr string (gdbpy_obj_to_string (result)); > - if (string == nullptr) > - gdbpy_handle_exception (); > - uiout->field_string (field_name, string.get ()); > - } > -} > - > -/* Serialize RESULT and print it in MI format to the current_uiout. > - > - This function handles the top-level result initially returned from the > - invoke method of the Python command implementation. At the top-level > - the result must be a dictionary. The values within this dictionary can > - be a wider range of types. Handling the values of the top-level > - dictionary is done by serialize_mi_result_1, see that function for more > - details. > - > - If anything goes wrong while parsing and printing the MI output then an > - error is thrown. */ > - > -static void > -serialize_mi_result (PyObject *result) > -{ > - /* At the top-level, the result must be a dictionary. */ > - > - if (!PyDict_Check (result)) > - gdbpy_error (_("Result from invoke must be a dictionary")); > - > - PyObject *key, *value; > - Py_ssize_t pos = 0; > - while (PyDict_Next (result, &pos, &key, &value)) > - { > - gdb::unique_xmalloc_ptr key_string > - (py_object_to_mi_key (key)); > - serialize_mi_result_1 (value, key_string.get ()); > - } > -} > - > /* Called when the MI command is invoked. PARSE contains the parsed > command line arguments from the user. */ > > @@ -388,7 +216,12 @@ mi_command_py::invoke (struct mi_parse *parse) const > gdbpy_handle_exception (); > > if (result != Py_None) > - serialize_mi_result (result.get ()); > + { > + /* At the top-level, the result must be a dictionary. */ > + if (!PyDict_Check (result.get ())) > + gdbpy_error (_("Result from invoke must be a dictionary")); > + serialize_mi_data (result.get ()); > + } > } > > /* See declaration above. */ > diff --git a/gdb/python/py-utils.c b/gdb/python/py-utils.c > index d5b07a80d82..72a232f251d 100644 > --- a/gdb/python/py-utils.c > +++ b/gdb/python/py-utils.c > @@ -597,3 +597,148 @@ gdbpy_fix_doc_string_indentation (gdb::unique_xmalloc_ptr doc) > > return doc; > } > + > +/* Convert KEY_OBJ into a string that can be used as a field name in MI > + output. KEY_OBJ must be a Python string object, and must only contain > + characters suitable for use as an MI field name. > + > + If KEY_OBJ is not a string, or if KEY_OBJ contains invalid characters, > + then an error is thrown. Otherwise, KEY_OBJ is converted to a string > + and returned. */ > + > +static gdb::unique_xmalloc_ptr > +py_object_to_mi_key (PyObject *key_obj) > +{ > + /* The key must be a string. */ > + if (!PyUnicode_Check (key_obj)) > + { > + gdbpy_ref<> key_repr (PyObject_Repr (key_obj)); > + gdb::unique_xmalloc_ptr key_repr_string; > + if (key_repr != nullptr) > + key_repr_string = python_string_to_target_string (key_repr.get ()); > + if (key_repr_string == nullptr) > + gdbpy_handle_exception (); > + > + gdbpy_error (_("non-string object used as key: %s"), > + key_repr_string.get ()); > + } > + > + gdb::unique_xmalloc_ptr key_string > + = python_string_to_target_string (key_obj); > + if (key_string == nullptr) > + gdbpy_handle_exception (); > + > + /* Predicate function, returns true if NAME is a valid field name for use > + in MI result output, otherwise, returns false. */ > + auto is_valid_key_name = [] (const char *name) -> bool > + { > + gdb_assert (name != nullptr); > + > + if (*name == '\0' || !isalpha (*name)) > + return false; > + > + for (; *name != '\0'; ++name) > + if (!isalnum (*name) && *name != '_' && *name != '-') > + return false; > + > + return true; > + }; > + > + if (!is_valid_key_name (key_string.get ())) > + { > + if (*key_string.get () == '\0') > + gdbpy_error (_("Invalid empty key in MI result")); > + else > + gdbpy_error (_("Invalid key in MI result: %s"), key_string.get ()); > + } > + > + return key_string; > +} > + > +static void > +serialize_mi_data_1 (PyObject *data, const char *field_name) The old version of this function `serialize_mi_result_1` had a comment, which is now missing. > +{ > + struct ui_out *uiout = current_uiout; > + > + if (PyDict_Check (data)) > + { > + PyObject *key, *value; > + Py_ssize_t pos = 0; > + ui_out_emit_tuple tuple_emitter (uiout, field_name); > + while (PyDict_Next (data, &pos, &key, &value)) > + { > + gdb::unique_xmalloc_ptr key_string > + (py_object_to_mi_key (key)); > + serialize_mi_data_1 (value, key_string.get ()); > + } > + } > + else if (PySequence_Check (data) && !PyUnicode_Check (data)) > + { > + ui_out_emit_list list_emitter (uiout, field_name); > + Py_ssize_t len = PySequence_Size (data); > + if (len == -1) > + gdbpy_handle_exception (); > + for (Py_ssize_t i = 0; i < len; ++i) > + { > + gdbpy_ref<> item (PySequence_ITEM (data, i)); > + if (item == nullptr) > + gdbpy_handle_exception (); > + serialize_mi_data_1 (item.get (), nullptr); > + } > + } > + else if (PyIter_Check (data)) > + { > + gdbpy_ref<> item; > + ui_out_emit_list list_emitter (uiout, field_name); > + while (true) > + { > + item.reset (PyIter_Next (data)); > + if (item == nullptr) > + { > + if (PyErr_Occurred () != nullptr) > + gdbpy_handle_exception (); > + break; > + } > + serialize_mi_data_1 (item.get (), nullptr); > + } > + } > + else > + { > + if (PyLong_Check (data)) > + { > + int overflow = 0; > + gdb_py_longest val = gdb_py_long_as_long_and_overflow (data, > + &overflow); > + if (PyErr_Occurred () != nullptr) > + gdbpy_handle_exception (); > + if (overflow == 0) > + { > + uiout->field_signed (field_name, val); > + return; > + } > + /* Fall through to the string case on overflow. */ > + } > + > + gdb::unique_xmalloc_ptr string (gdbpy_obj_to_string (data)); > + if (string == nullptr) > + gdbpy_handle_exception (); > + uiout->field_string (field_name, string.get ()); > + } > +} > + > +/* See python-internal.h. */ > + > +void > +serialize_mi_data (PyObject *data) > +{ > + gdb_assert (PyDict_Check (data)); > + > + PyObject *key, *value; > + Py_ssize_t pos = 0; > + while (PyDict_Next (data, &pos, &key, &value)) > + { > + gdb::unique_xmalloc_ptr key_string > + (py_object_to_mi_key (key)); > + serialize_mi_data_1 (value, key_string.get ()); > + } > +} > diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h > index 93217375cc5..3f53b0ab6f0 100644 > --- a/gdb/python/python-internal.h > +++ b/gdb/python/python-internal.h > @@ -486,6 +486,21 @@ struct gdbarch *arch_object_to_gdbarch (PyObject *obj); > extern PyObject *gdbpy_execute_mi_command (PyObject *self, PyObject *args, > PyObject *kw); > > +/* Serialize DATA and print it in MI format to the current_uiout. > + > + This function handles the top-level result initially returned from the > + invoke method of the Python command implementation. At the top-level > + the data must be a dictionary. Caller of this function is responsible > + for ensuring that. The values within this dictionary can > + be a wider range of types. Handling the values of the top-level > + dictionary is done by serialize_mi_data_1, see that function for more > + details. Given the change coming in the next commit, I wonder if this comment should be reworded to avoid talking about the 'invoke method'. Maybe just say how it formats DATA as an MI record. > + > + If anything goes wrong while parsing and printing the MI output then an > + error is thrown. */ > + > +extern void serialize_mi_data (PyObject *result); s/result/data/ here. Thanks, Andrew > + > /* Convert Python object OBJ to a program_space pointer. OBJ must be a > gdb.Progspace reference. Return nullptr if the gdb.Progspace is not > valid (see gdb.Progspace.is_valid), otherwise return the program_space > -- > 2.40.1