[dpdk-dev] [PATCH] usertools: replace unsafe input function

[Richardson, Bruce]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, April 3, 2019 3:59 PM
> To: Richardson, Bruce <bruce.richardson at intel.com>
> Cc: Sirvys, Andrius <andrius.sirvys at intel.com>; dev at dpdk.org; Laatz, Kevin
> <kevin.laatz at intel.com>; stable at dpdk.org; ciara.power at intel.com
> Subject: Re: [dpdk-dev] [PATCH] usertools: replace unsafe input function
> 
> On 03-Apr-19 3:48 PM, Burakov, Anatoly wrote:
> > On 03-Apr-19 3:29 PM, Burakov, Anatoly wrote:
> >> On 03-Apr-19 2:30 PM, Bruce Richardson wrote:
> >>> On Wed, Apr 03, 2019 at 11:44:40AM +0100, Burakov, Anatoly wrote:
> >>>> On 20-Mar-19 4:43 PM, Andrius Sirvys wrote:
> >>>>> LGTM static code analysis tool reports that the function 'input'
> >>>>> is unsafe. Changed to use raw_input which then converts it using
> >>>>> ast.literal_eval() which is safe.
> >>>>>
> >>>>> Fixes: d1b94da4a4e0 ("usertools: add client script for telemetry")
> >>>>> Cc: ciara.power at intel.com
> >>>>>
> >>>>> Signed-off-by: Andrius Sirvys <andrius.sirvys at intel.com>
> >>>>> ---
> >>>>>    usertools/dpdk-telemetry-client.py | 4 ++--
> >>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/usertools/dpdk-telemetry-client.py
> >>>>> b/usertools/dpdk-telemetry-client.py
> >>>>> index ce0c7a9..c3ba77d 100755
> >>>>> --- a/usertools/dpdk-telemetry-client.py
> >>>>> +++ b/usertools/dpdk-telemetry-client.py
> >>>>> @@ -72,7 +72,7 @@ def requestMetrics(self): # Requests metrics for
> >>>>> given client
> >>>>>        def repeatedlyRequestMetrics(self, sleep_time): #
> >>>>> Recursively requests metrics for given client
> >>>>>            print("\nPlease enter the number of times you'd like to
> >>>>> continuously request Metrics:")
> >>>>> -        n_requests = int(input("\n:"))
> >>>>> +        n_requests = int(ast.literal_eval(raw_input("\n:")))
> >>>>>            print("\033[F") #Removes the user input from screen,
> >>>>> cleans it up
> >>>>>            print("\033[K")
> >>>>>            for i in range(n_requests):
> >>>>> @@ -87,7 +87,7 @@ def interactiveMenu(self, sleep_time): # Creates
> >>>>> Interactive menu within the scr
> >>>>>                print("[3] Unregister client")
> >>>>>                try:
> >>>>> -                self.choice = int(input("\n:"))
> >>>>> +                self.choice =
> >>>>> +int(ast.literal_eval(raw_input("\n:")))
> >>>>>                    print("\033[F") #Removes the user input for
> >>>>> screen, cleans it up
> >>>>>                    print("\033[K")
> >>>>>                    if self.choice == 1:
> >>>>>
> >>>>
> >>>> raw_input doesn't exist in Python 3.
> >>>>
> >>>> Perhaps you should do this at the top of the script:
> >>>>
> >>>> try:
> >>>>      raw_input  # Python 2
> >>>> except NameError:
> >>>>      raw_input = input  # Python 3
> >>>>
> >>>> That way, all calls to raw_input will call the intended function.
> >>>>
> >>>
> >>> The suggested way in the python docs is a little different:
> >>>
> >>> https://python-future.org/compatible_idioms.html#raw_input()
> >>>
> >>
> >> Or that :)
> >>
> >
> > Actually, this requires a dependency. "future" appears to be
> > preinstalled on most distributions, but "builtins" isn't. In fact, i
> > can't even find it in pip.
> >
> 
> So, it does work on python2 and python3, however it indeed requires a
> "future" package to be install through pip, which makes it an external
> dependency. The way i have quoted above doesn't require the 'future'
> package. I'm of no opinion on whether we should make the 'future'
> package a requirement for all of our Python code, however if we were to
> use this, it adds one extra step during setup and thus should be
> documented.
> 

Going with your way is fine, and it does seem cleaner to me.

However, we should also question if we need to still support python2? 
Even if we do right now, we probably can drop support
for it at some point in the very near future.

/Bruce