From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id B4B2450ADF1; Wed, 14 Aug 2024 22:34:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B4B2450ADF1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1723664050; bh=Q3A03McM7PQAnNi8RuVkLrwjrAgYN+W6w+sjzLzeh8E=; h=Date:To:References:Cc:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BJIpRndk5cXRK7jY9k0vzGo4A/3onpqbNKt9aI5YoBeV4aRxkEeGM6fOtPPhwvD5O BRQFt4R3NCCw7lX8DWbNzGajecnS8d3boFHt/M8pUiEvukzppVNSZsfZx9cXRENmo+ I4OQGmFV2GwbV44R13YM86YQaatvx/t3oQe5sV48= Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [95.163.41.87]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BB31E50ADF1 for ; Wed, 14 Aug 2024 22:34:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org BB31E50ADF1 Received: by smtp51.i.mail.ru with esmtpa (envelope-from ) id 1seJl1-0000000FxHk-0fNz; Wed, 14 Aug 2024 22:34:07 +0300 Content-Type: multipart/alternative; boundary="------------RJWJUQG9dq4CHUvtoVJ4LOIY" Message-ID: <75025f3a-47b2-4094-8aea-8762fdd785ac@vk.team> Date: Wed, 14 Aug 2024 22:34:06 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Maxim Kokryashkin References: <13501abba00ac3e072284d36a531c721e279722f.1712182830.git.m.kokryashkin@tarantool.org> Content-Language: en-US Cc: tarantool-patches@dev.tarantool.org In-Reply-To: <13501abba00ac3e072284d36a531c721e279722f.1712182830.git.m.kokryashkin@tarantool.org> X-Mailru-Src: smtp X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD9CB0CA91FF61D77F0FECC69FE2B0083BEDB51CAAD409268F5182A05F5380850404C228DA9ACA6FE274547A46086FBE153411046492FDDF806480424AB6DDE7EC29B544B28A0F7CDD0A39B5B018D1B5ADE X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7BCC85671EC7A750CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637652CD06254D2F21C8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8F0A4717A56BD13947BEE7748BB823034E1D91D6B25978FD920879F7C8C5043D14489FFFB0AA5F4BF1661749BA6B977359735652A29929C6C4AD6D5ED66289B5278DA827A17800CE7D954429C29F3E183D32BA5DBAC0009BE395957E7521B51C2330BD67F2E7D9AF1090A508E0FED6299176DF2183F8FC7C0F01703C1D0110F50CD04E86FAF290E2DB606B96278B59C421DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B640A672C58500DF29089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5A3B333995E14B1FE5002B1117B3ED696950249520CB6165FF5FEB6EB1EB183FD823CB91A9FED034534781492E4B8EEAD37F46C620FF2CAEEBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742DC8270968E61249B1004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3455AC8BF8E3153BA08BB9F22F57AD30A0A5AFE2A704EFD920C6059A0EE049C9F2130C156A7FBDABE91D7E09C32AA3244C6E7FED5180DBD3A37215182CA7F288602110682B314C78AAEA455F16B58544A2E30DDF7C44BCB90D965026E5D17F6739903BEA1B06666FAA1044F86CB46F436F3FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojnMg09N3zL/gWJHCiGln6Og== X-Mailru-Sender: 0A26D9779F8DDEABC9EDB3742E1D2314E2AFB982EEF83F6EAA70CB78A12CDBB583833D0826AD12D17392DF524794EFEE4CA08BFBBB45AAA72C22B24405C8F0CCB7331131FBF1F034D3A388E327D6469BB4C443D66B1C03FA314DF935DB795A1AEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit v6 1/2] debug: generalized extension X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mikhail Elhimov via Tarantool-patches Reply-To: Mikhail Elhimov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------RJWJUQG9dq4CHUvtoVJ4LOIY Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Maxim! Thanks for the patch! Please consider my comments below. On 04.04.2024 01:21, Maxim Kokryashkin via Tarantool-patches wrote: > +class Debugger(object): > + def __init__(self): > + self.GDB = False > + self.LLDB = False > + > + debuggers = { > + 'gdb': lambda lib: True, > + 'lldb': lambda lib: lib.debugger is not None, > + } > + for name, healthcheck in debuggers.items(): > + lib = None > + try: > + lib = import_module(name) > + if healthcheck(lib): > + setattr(self, name.upper(), True) > + globals()[name] = lib > + self.name = name > + except Exception: > + continue > + > + assert self.LLDB != self.GDB I'd suggest to use two separate implementations of Debugger interface for GDB and LLDB, so you would not need all these checking (like `if self.LLDB`) in every single method of all-in-one implementation. With this approach it seems any initial setup that is specific to certain debugger (like setup of event_connect/event_disconnect handlers for GDB) could be done as a part of corresponding __init__ method. -- Best regards, Mikhail Elhimov --------------RJWJUQG9dq4CHUvtoVJ4LOIY Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit 
Hi, Maxim!
Thanks for the patch!
Please consider my comments below.

On 04.04.2024 01:21, Maxim Kokryashkin via Tarantool-patches wrote:
<snipped>
+class Debugger(object):
+    def __init__(self):
+        self.GDB = False
+        self.LLDB = False
+
+        debuggers = {
+            'gdb': lambda lib: True,
+            'lldb': lambda lib: lib.debugger is not None,
+        }
+        for name, healthcheck in debuggers.items():
+            lib = None
+            try:
+                lib = import_module(name)
+                if healthcheck(lib):
+                    setattr(self, name.upper(), True)
+                    globals()[name] = lib
+                    self.name = name
+            except Exception:
+                continue
+
+        assert self.LLDB != self.GDB

I'd suggest to use two separate implementations of Debugger interface for GDB and LLDB, so you would not need all these checking (like `if self.LLDB`) in every single method of all-in-one implementation.

With this approach it seems any initial setup that is specific to certain debugger (like setup of event_connect/event_disconnect handlers for GDB) could be done as a part of corresponding __init__ method.

<snipped>
-- 
Best regards,
Mikhail Elhimov
--------------RJWJUQG9dq4CHUvtoVJ4LOIY--