Cwe caller #40
Cwe caller #40
Conversation
src/caller/cwe_checker.ml
Outdated
| let main () : int = | ||
| match Array.length Sys.argv with | ||
| | 1 -> Sys.command ("bap " ^ Sys.argv.(1) ^ " --pass=cwe-checker ") | ||
| | _ -> | ||
| match process_flags with | ||
| | None -> 1 | ||
| | Some [] -> Sys.command ("bap " ^ Sys.argv.(1) ^ " --pass=cwe-checker ") | ||
| | Some flags -> Sys.command ("bap " ^ Sys.argv.(1) ^ " --pass=cwe-checker " ^ setup_flags flags) |
There was a problem hiding this comment.
if no flags are given or only the flag --help, we should print a help message. Oh, in that regard we should also implement a --version flag!
src/caller/helper_functions.mli
Outdated
| @@ -0,0 1,20 @@ | |||
| (* Removes the nth element of a list *) | |||
There was a problem hiding this comment.
The comments in .mli files should be documentation comments. Documentation comments start with (** instead of just (* and are parsed by odoc to generate the online documentation.
src/caller/helper_functions.ml
Outdated
| (* Just for type conversion: Returns an empty string if none, else the string *) | ||
| let get_default_string (in_option : string option) : string = | ||
| match in_option with | ||
| | None -> "" | ||
| | Some s -> s |
There was a problem hiding this comment.
There is already a method for Options which does the same.
changed comments to doc comments removed suffix check for output files
|
The command line flags and parameters are now defined in the Cwe_checker_core.Main module. @mellowCS Could you refactor your code to read them from there? We should also print a custom help message for the --help flag. |
…om Cwe_checker_core.Main - added a function for the help message which is either called when no arguments are given, when the user made an error in the input or when the help flag is set. - removed the json reader function
Added function to get first element of tuple
…and get_second generalized get_first and get_second to support any input type
caller/cwe_checker.ml
Outdated
| match Sys.is_directory path with | ||
| | false -> generate_output_file path ~file:"" () | ||
| | true -> build_path path |
There was a problem hiding this comment.
If the user provides a name of a non-existing file as output path, just pass it through to bap without modification. The filename should not be modified (i.e. no timestamp) in this case to simplify writing scripts with the result from the cwe_checker as input.
There was a problem hiding this comment.
Can the user pass non-existing folders as well? If not, I would check if the given path is valid.
caller/cwe_checker.ml
Outdated
| with | ||
| | _ -> raise (NoOutputFileException "No output file provided. If -out flag is set please provide an out file.") |
There was a problem hiding this comment.
This captures the exception out_path can throw and replaces it with another exception. I don't understand why you want to do that here.
caller/cwe_checker.ml
Outdated
| let check_for_help (flags: string list) : string list = | ||
| if (Stdlib.List.mem "-help" flags) then ( | ||
| help (); | ||
| remove_string flags "-help" | ||
| ) else flags |
There was a problem hiding this comment.
We should also check for "--help". Also, the expected behavior for the help flag is to just print the help message and then exit without doing anything else.
|
Correction: We do want to keep the value checks for parameters here. My mistake. I will move them afterwards to the Main module so that we can use the same checks here and in the Main module. |
…s_params - changed return types of checks from bool to unit and replaced them with exceptions - added a check for the binary path - some more minor type changes - removed unused functions - transferred changes of cwe_checker.ml to the mli file respectively - fixed: binary path's position in command has to be first. -> Now it can be at any position
|
Looks good. I will add some refactoring of the main module and after that we can merge it. |
No description provided.