Re: [Tarantool-patches] [PATCH 1/2] gdb: adjust to support python2 (centos 7)

[Mikhail Elhimov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Igor!

Thanks for the review!

On 31.08.2022 17:47, Igor Munkin wrote:
> Misha,
>
> Thanks for the patch! Almost LGTM after your fixes for Sergey comments:
> please, consider the last nit, I found.
>
> On 23.07.22, Mikhail Elhimov via Tarantool-patches wrote:
>> Stop using unpacking arguments within list initialization as python2
>> only supports it in function call
>>
>> fixes tarantool/luajit#7458
>> ---
>>   src/luajit-gdb.py | 22 ++++++++++++----------
>>   1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
>> index baf66f66..1e9a96fb 100644
>> --- a/src/luajit-gdb.py
>> +++ b/src/luajit-gdb.py
>> @@ -432,15 +432,16 @@ def dump_stack(L, base=None, top=None):
>>       maxstack = mref('TValue *', L['maxstack'])
>>       red = 5 + 2 * LJ_FR2
>>   
>> -    dump = '\n'.join([
>> -        '{padding} Red zone: {nredslots: >2} slots {padding}'.format(
>> -            padding = '-' * len(PADDING),
>> -            nredslots = red,
>> -        ),
>> -        *(
>> -            dump_stack_slot(L, maxstack + offset, base, top, '')
>> -                for offset in range(red, 0, -1)
>> -        ),
>> +    dump = []
> Please move this line to the "definition" section above to save the
> original structure.

Fixed.

I personally prefer to do similar operations (populating of `dump` list) 
in a similar way (call appropriate "populating" method). For me 
populating-over-initialization is not the "similar" way here, and I'd 
use it here only if I could initialize whole list at once (as it was 
before). Anyway, if you find that it saves the original structure then 
let it be )

>> +    dump.append('{padding} Red zone: {nredslots: >2} slots {padding}'.format(
>> +        padding = '-' * len(PADDING),
>> +        nredslots = red,
>> +    ))
>> +    dump.extend([
>> +        dump_stack_slot(L, maxstack + offset, base, top, '')
>> +            for offset in range(red, 0, -1)
>> +    ])
>> +    dump.extend([
>>           '{padding} Stack: {nstackslots: >5} slots {padding}'.format(
>>               padding = '-' * len(PADDING),
>>               nstackslots = int((tou64(maxstack) - tou64(stack)) >> 3),
>> @@ -451,7 +452,8 @@ def dump_stack(L, base=None, top=None):
>>               end = strx64(maxstack - 1),
>>               nfreeslots = int((tou64(maxstack) - tou64(top) - 8) >> 3),
>>           ),
>> -    ]) + '\n'
>> +    ])
>> +    dump = '\n'.join(dump) + '\n'
>>   
>>       slot = top
>>       framelink = base - (1 + LJ_FR2)
>> -- 
>> 2.34.1
>>
===================================================

diff --git a/src/luajit-gdb.py b/src/luajit-gdb.py
index baf66f66..9846f3a2 100644
--- a/src/luajit-gdb.py
+++ b/src/luajit-gdb.py
@@ -432,15 +432,17 @@ def dump_stack(L, base=None, top=None):
      maxstack = mref('TValue *', L['maxstack'])
      red = 5 + 2 * LJ_FR2

-    dump = '\n'.join([
+    dump = [
          '{padding} Red zone: {nredslots: >2} slots {padding}'.format(
              padding = '-' * len(PADDING),
              nredslots = red,
          ),
-        *(
-            dump_stack_slot(L, maxstack + offset, base, top, '')
-                for offset in range(red, 0, -1)
-        ),
+    ]
+    dump.extend([
+        dump_stack_slot(L, maxstack + offset, base, top, '')
+            for offset in range(red, 0, -1)
+    ])
+    dump.extend([
          '{padding} Stack: {nstackslots: >5} slots {padding}'.format(
              padding = '-' * len(PADDING),
              nstackslots = int((tou64(maxstack) - tou64(stack)) >> 3),
@@ -451,7 +453,8 @@ def dump_stack(L, base=None, top=None):
              end = strx64(maxstack - 1),
              nfreeslots = int((tou64(maxstack) - tou64(top) - 8) >> 3),
          ),
-    ]) + '\n'
+    ])
+    dump = '\n'.join(dump) + '\n'

      slot = top
      framelink = base - (1 + LJ_FR2)

===================================================

-- 
Best regards,
Mikhail Elhimov