From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36478 invoked by alias); 28 May 2019 20:35:44 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 36464 invoked by uid 89); 28 May 2019 20:35:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-8.5 required=5.0 tests=AWL,BAYES_00,SPF_PASS autolearn=ham version=3.3.1 spammy=H*r:user, H*r:may, H*r:forged X-HELO: relay.fit.cvut.cz Received: from relay.fit.cvut.cz (HELO relay.fit.cvut.cz) (147.32.232.237) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 28 May 2019 20:35:42 +0000 Received: from imap.fit.cvut.cz (imap.fit.cvut.cz [IPv6:2001:718:2:2901:0:0:0:238]) by relay.fit.cvut.cz (8.15.2/8.15.2) with ESMTPS id x4SKZYmj035305 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Tue, 28 May 2019 22:35:36 +0200 (CEST) (envelope-from jan.vrany@fit.cvut.cz) Received: from sao (02791bac.bb.sky.com [2.121.27.172] (may be forged)) (authenticated bits=0 as user vranyj1) by imap.fit.cvut.cz (8.15.2/8.15.2) with ESMTPSA id x4SKZVsk021714 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT); Tue, 28 May 2019 22:35:32 +0200 (CEST) (envelope-from jan.vrany@fit.cvut.cz) Message-ID: Subject: Re: [PATCH v2 3/5] Create MI commands using python. From: Jan Vrany To: Simon Marchi , gdb-patches Date: Tue, 28 May 2019 20:35:00 -0000 In-Reply-To: <128d0fc3-4364-0cab-f03d-45ed7d3eda32@simark.ca> References: <20190418152337.6376-1-jan.vrany@fit.cvut.cz> <20190514112418.24091-4-jan.vrany@fit.cvut.cz> <128d0fc3-4364-0cab-f03d-45ed7d3eda32@simark.ca> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.30.5-1 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-05/txt/msg00602.txt.bz2 Hi, thanks a lot! I agree with most of comments and fixed them already in upcoming v3 patch. For the rest, sae below. > > > + { > > > + /* Since PyList_SetItem steals the reference, we don't use > > > + * gdbpy_ref<> to hold on arg string. */ > > > + PyObject *str = PyUnicode_Decode (parse->argv[i], strlen (parse->argv[i]), > > > + host_charset (), NULL); > > > + if (PyList_SetItem (argobj.get (), i, str) != 0) > > > > Just wondering, because I know that unicode handling is very different between > > Python 2 and 3: did you test with both Python versions? > > No. I will do that before sending v3. > > > + gdbpy_ref<> result ( > > > + PyObject_CallMethodObjArgs (obj, invoke_cst, argobj.get (), NULL)); > > > + if (PyErr_Occurred () != NULL) > > > + { > > > + gdbpy_err_fetch ex; > > > + gdb::unique_xmalloc_ptr ex_msg (ex.to_string ()); > > > + > > > + if (ex_msg == NULL || *ex_msg == '\0') > > > + error (_("-%s: failed to execute command"), name ().c_str ()); > > > + else > > > + error (_("-%s: %s"), name ().c_str (), ex_msg.get ()); > > > + } > > > + else if (result != nullptr) > > > + { > > > + if (Py_None != result) > > > + parse_mi_result (result.get (), "result"); > > > + } > > > + else > > > + { > > > + error ( > > > + _("-%s: command invoke() method returned NULL but no python exception " > > > + "is set"), > > > + name ().c_str ()); > > > > I am curious, did you actually hit this case, where no Python exception was > > raised and invoke returned NULL? I thought the two were pretty coupled > > (if return value is NULL, it means there's an exception raise and vice versa). > > Yes, it looks like. I guess I took this code from Didier. I removed the else branch in v3. > > > + void invoke (struct mi_parse *parse) override; > > > > This should be private, like in the previous patch. > > Ah, this is "bigger" problem, we should override do_invoke() which is declared protected in superclass. Again,fixed in v3. Thanks a lot! Jan