[PATCH v2] elf: Fix tst-relro-symbols.py argument passing

[Adhemerval Zanella Netto]

On 15/12/22 16:36, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> Current scheme only consideres the first argument for both --required
>> and --optional, where the idea is to append a new item.
>>
>> Checked on x86_64-linux-gnu.
>> ---
>>  elf/tst-relro-symbols.py | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/elf/tst-relro-symbols.py b/elf/tst-relro-symbols.py
>> index 368ea3349f..a572a47148 100644
>> --- a/elf/tst-relro-symbols.py
>> +++ b/elf/tst-relro-symbols.py
>> @@ -56,10 +56,10 @@ def get_parser():
>>      """Return an argument parser for this script."""
>>      parser = argparse.ArgumentParser(description=__doc__)
>>      parser.add_argument('object', help='path to object file to check')
>> -    parser.add_argument('--required', metavar='NAME', default=(),
>> -                        help='required symbol names', nargs='*')
>> -    parser.add_argument('--optional', metavar='NAME', default=(),
>> -                        help='required symbol names', nargs='*')
>> +    parser.add_argument('--required', metavar='NAME', action='append', default=[],
>> +                        help='required symbol names')
>> +    parser.add_argument('--optional', metavar='NAME', action='append', default=[],
> 
> Nit: Like length limit exceeded.

Right, I will fix it.

> 
>> +                        help='required symbol names')
>>      return parser
>>  
>>  def main(argv):
> 
> Despite the use of [] here, there does not seem to be a sharing hazard:
> 
> import argparse
> parser = argparse.ArgumentParser()
> parser.add_argument('--foo', action='append', default=[])
> print(parser.parse_args('--foo 1 --foo 2'.split()))
> print(parser.parse_args('--foo 3 --foo 4'.split()))
> 
> Prints:
> 
> Namespace(foo=['1', '2'])
> Namespace(foo=['3', '4'])
> 
> I would have expected:
> 
> Namespace(foo=['1', '2'])
> Namespace(foo=['1', '2', '3', '4'])

For state tracking you need to pass a namespace, as described by documentation [1].

import argparse

class State:
   pass

state = State()
parser = argparse.ArgumentParser()
parser.add_argument('--foo', action='append', default=[])
parser.parse_args('--foo 1 --foo 2'.split(), namespace=state)
print(state.foo)
parser.parse_args('--foo 3 --foo 4'.split(), namespace=state)
print(state.foo)

It prints:

['1', '2']
['1', '2', '3', '4']

> 
> Either way, given that the parser is only used once, that doesn't really
> matter.  So we can implement it this way even though the documentation
> is ambiguous.
> 
> Reviewed-by: Florian Weimer <fweimer@redhat.com>
> 
> Thanks,> Florian

[1] https://docs.python.org/3/library/argparse.html#namespace