ubi_reader icon indicating copy to clipboard operation
ubi_reader copied to clipboard

Platform agnostic CLI

Open AT0myks opened this issue 3 years ago • 3 comments

Here's the new PR, if I didn't make any mistakes the scripts should behave exactly the same as before.

I didn't update the readme to reflect the changes because I'm not sure how you would like it to be edited.

AT0myks avatar Nov 28 '22 19:11 AT0myks

Hi AT0myks

Thank you, I'm a little busy at the moment, and this will take some time to review and change my test set up, but I will get to it.

-Jason

jrspruitt avatar Nov 29 '22 17:11 jrspruitt

Hi AT0myks,

Just an update, I've been working on this. I took the idea a little further and made a UIParser class, where the most common cli arguments could go, along with handling the checks and logic for the variables they populate. I got a couple of the scripts working, ubireader_extract_files and ubireader_extract_images. It gets rid of a lot of duplicate code, but does require function calls to add args. I think you could still use this from a script. Any thoughts?

platform-agnostic-cli

-Jason

jrspruitt avatar Dec 05 '22 09:12 jrspruitt

That looks pretty good to me. Removing the duplicate code at the beginning of each function was on my list. I don't see any obvious problem when using this from a script.

Here are my thoughts:

I think usage and description could be UIParser arguments:

class UIParser(object):
    def __init__(self, usage=None, description=None):
        self._p = argparse.ArgumentParser(add_help=False, usage=usage, description=description)

Use property decorators:

def set_usage(self, usage):
    self._p.usage = usage
def get_usage(self):
    return self._p.usage
usage = property(get_usage, set_usage)

becomes

@property
def usage(self):
    return self._p.usage
@usage.setter
def usage(self, usage):
    self._p.usage = usage

In UIParser there could be a method like add_common_args which adds the 10 common arguments so that, for example in extract_files, you can replace

ui.arg_keep_permissions()
ui.arg_log()
ui.arg_verbose_log()
ui.arg_peb_size()
ui.arg_start_offset()
ui.arg_end_offset()
ui.arg_guess_offset()
ui.arg_warn_only_block_read_errors()
ui.arg_ignore_block_header_errors()
ui.arg_u_boot_fix()
ui.arg_output_dir()
ui.arg_filepath()

by

ui.add_common_args()
ui.arg_keep_permissions()
ui.arg_output_dir()

It also helps identifying the unique arguments of each parser and makes it a bit easier to add common arguments later.

filepath being a required positional argument, I think it should be an argument of parse_args:

def parse_args(self, filepath, *args, **kwargs):

and then also of all parser functions

def extract_files(filepath, *args, **kwargs):

This way in another script you can call the functions with the default arguments with extract_files("path") instead of having to give it as a keyword argument like extract_files(filepath="path").

AT0myks avatar Dec 05 '22 18:12 AT0myks