Refactor output modes to use enum

[XVilka]

Currently you can often see in the code the following pattern:

RZ_API void rz_core_task_list(RzCore *core, int mode) {
    if (mode == 'j') {
         ...
    }
}

In some cases it looks like that:

RZ_API int rz_core_disasm_pde(RzCore *core, int nb_opcodes, int mode) {
    if (mode == RZ_MODE_JSON) {
        ...
    }
}

Sometimes there is no "mode" word, and it looks like that:

RZ_API void rz_analysis_xrefs_list(RzAnalysis *analysis, int rad) {
    ...
    if (rad == 'j') {
        ...
    }

I recommend to search for int mode and char mode patterns in the code.

It prevents catching some errors at the compilation time and reduces the readability. Should be refactored to use enum instead, across the whole Rizin code.

After everything ported, the following hardcoded values should be removed from librz/include/rz_types.h:

#define RZ_MODE_PRINT     0x000
#define RZ_MODE_RIZINCMD  0x001
#define RZ_MODE_SET       0x002
#define RZ_MODE_SIMPLE    0x004
#define RZ_MODE_JSON      0x008
#define RZ_MODE_ARRAY     0x010
#define RZ_MODE_SIMPLEST  0x020
#define RZ_MODE_CLASSDUMP 0x040
#define RZ_MODE_EQUAL     0x080

See, for example:

librz/util/range.c
314:int rz_range_list(RRange *rgs, int rad) {

librz/io/io_cache.c
93:RZ_API bool rz_io_cache_list(RzIO *io, int rad) {

librz/main/rz-bin.c
63:static RzOutputMode rad2outputmode(int rad) {
471:static int rabin_do_operation(RzBin *bin, const char *op, int rad, const char *output, const char *file) {
622:static void __listPlugins(RzBin *bin, const char *plugin_name, PJ *pj, int rad) {
656:    int rad = 0;

librz/core/core_private.h
123:RZ_IPI bool rz_core_debug_reg_list(RzCore *core, int type, int size, bool skip_covered, PJ *pj, int rad, const char *use_color);

librz/core/cdebug.c
300:RZ_IPI bool rz_core_debug_reg_list(RzCore *core, int type, int size, bool skip_covered, PJ *pj, int rad, const char *use_color) {

librz/core/cmd/cmd_flag.c
734:    int rad;
759:static void print_function_labels_for(RzAnalysisFunction *fcn, int rad, PJ *pj) {
772:static void print_function_labels(RzAnalysis *analysis, RzAnalysisFunction *fcn, int rad) {

librz/core/canalysis.c
4157:static bool found_xref(RzCore *core, ut64 at, ut64 xref_to, RzAnalysisXRefType type, PJ *pj, int rad, int cfg_debug, bool cfg_analysis_strings) {
4225:RZ_API int rz_core_analysis_search_xrefs(RzCore *core, ut64 from, ut64 to, PJ *pj, int rad) {

librz/core/cmd/cmd_analysis.c
7005:   int rad;

librz/debug/ddesc.c
71:RZ_API int rz_debug_desc_list(RzDebug *dbg, int rad) {

librz/flag/flag.c
374:RZ_API void rz_flag_list(RzFlag *f, int rad, const char *pfx) {

librz/debug/p/bfvm.c
273:RZ_API void bfvm_show_regs(BfvmCPU *c, int rad) {
289:RZ_API void bfvm_maps(BfvmCPU *c, int rad) {

librz/cons/pal.c
526:RZ_API void rz_cons_pal_list(int rad, const char *arg) {

librz/include/rz_flag.h
97:RZ_API void rz_flag_list(RzFlag *f, int rad, const char *pfx);

librz/include/rz_debug.h
518:RZ_API int rz_debug_desc_list(RzDebug *dbg, int rad);

librz/include/rz_cons.h
953:RZ_API void rz_cons_pal_list(int rad, const char *arg);

librz/include/rz_analysis.h
55:     int rad;

librz/include/rz_io.h
394:RZ_API bool rz_io_cache_list(RzIO *io, int rad);

librz/include/rz_core.h
670:RZ_API int rz_core_analysis_search_xrefs(RzCore *core, ut64 from, ut64 to, PJ *pj, int rad);

librz/include/rz_util/rz_range.h
35:RZ_API int rz_range_list(RRange *rgs, int rad);

[ret2libc]

Please note that there is RzOutputMode that is used in newshell commands.

[valdaarhun]

Hi. I am currently working on this issue as a microtask for gsoc this year. I was going through the code and there are several questions that I would like to ask.

1. My understanding is that in statements like if (mode ==’j’), ‘j’ needs to be replaced with an enum constant. Is that correct? Basically should all literals in if/switch statements be replaced with an enum constant?

2. If the answer to the above question is yes, does that mean I will have to declare new enum structures in a file like “rz_types.h” or should I use the constants in RzOutputMode?

3. I also didn’t understand why lines 21 through 29 are #defines and not in an enum structure.

4. Do ‘r’ and ‘q’ stand for ‘RZ_OUTPUT_MODE_RIZIN’ and ‘RZ_OUTPUT_MODE_QUIET’ respectively. Do you know where I could find a list of abbreviations? Are they the same as the output modes given by the p command in rizin?

[XVilka]

@valdaarhun

1. Yes, and the argument type should be of RzOutputMode
2. Use RzOutputMode enum
3. Just ignore those
4. Precisely, "*" also means "RIZIN" usually, "k" means "SDB", "l" means "LONG".

See the example of RZ_IPI RzCmdStatus rz_seek_history_list_handler(RzCore *core, int argc, const char **argv, RzOutputMode mode) in librz/core/cmd_seek.c

[valdaarhun]

@XVilka Thanks for the clarifications! I'll start working on this immediately.

[valdaarhun]

There are a few more modes that I have come across. Could you please let me know what they are? They are 'c', 'x' and 'w' in this switch case and 'm' in this statement

[ret2libc]

There are a few more modes that I have come across. Could you please let me know what they are? They are 'c', 'x' and 'w' in this switch case and 'm' in this statement

There are some common modes, that are explained in RzOutputMode. Other commands might have various suffixes, but they are not generic enough to use them everywhere. So for those cases we won't use just RzOutputMode.

[valdaarhun]

In that case, should I leave the ones that aren't described in RzOutputMode untouched?

[ret2libc]

In that case, should I leave the ones that aren't described in RzOutputMode untouched?

Yes

[valdaarhun]

In some functions (for example in this one), mode is being initialized to a number for example int mode = 0. Should I change this to RzOutputMode mode = RZ_OUTPUT_MODE_STANDARD?

Also, should RZ_MODE_RIZINCMD and RZ_MODE_SIMPLE be changed to RZ_OUTPUT_MODE_RIZIN and RZ_OUTPUT_MODE_STANDARD respectively?

[ret2libc]

@valdaarhun SIMPLE is RZ_OUTPUT_MODE_QUIET, while RIZINCMD is RZ_OUTPUT_MODE_RIZIN.

[valdaarhun]

I tried rebuilding and recompiling the project after making the changes and got a compilation error.

In this switch case, there's case 2 on line 181 and case 'j' on line 245. I can't simply change case 'j' to case RZ_OUTPUT_MODE_JSON because of the definition RZ_OUTPUT_MODE_JSON = 1 << 1 in the enum.

Should I leave case 'j' as it is?

[wargio]

when you convert the command you have to add a small fix to the old shell. essentially instead of passing to mode a char you will have to pass RZ_OUTPUT_MODE_<something> and handle 'j' from where the (for example) rz_core_task_list is being called on old shell

[valdaarhun]

@wargio I have understood what you mean. So basically, for example, in functions that have mode as an argument, I need to pass for example RZ_OUTPUT_MODE_JSON instead of 'j'.

But there are some functions in which it checks the value of mode(for example if (mode == 0) or if (mode == 'j')). There are a few places where switch case has been used to do the checking(like here). In this switch case, there is a case 2 block and a case 'j' block. If I change case 'j' to case RZ_OUTPUT_MODE_JSON, I will get a compilation error because RZ_OUTPUT_MODE_JSON will be treated as 1 << 1 (which is equal to 2) and this will clash with the case 2 block. So what do I do in such a situation?

[wargio]

you can still change that from case 'j': to case RZ_OUTPUT_MODE_JSON.

[wargio]

You also need to change int mode into RzOutputMode mode

[valdaarhun]

You also need to change int mode into RzOutputMode mode

Yeah, I have done this.

you can still change that from case 'j': to case RZ_OUTPUT_MODE_JSON.

I tried doing this. But then I get this error:

[valdaarhun]

Hi. I have almost completed refactoring the code base. There were just a few more questions that I wanted to ask.

1. If I am not mistaken int mode = 0 should be changed to RZ_OUTPUT_MODE_QUIET, right? Similarly, what should mode = 1, mode = 2 and mode = 3 be changed to?

2. Just as RZ_MODE_SIMPLEis RZ_OUTPUT_MODE_QUIET, what is RZ_MODE_SIMPLEST?

3. Should char mode = ' ' and char mode = '\0' be changed to RZ_OUTPUT_MODE_STANDARD?

[wargio]

can you please open a PR (draft mode) so we can inspect the code? it's hard to understand what you are talking about!

[valdaarhun]

@wargio Yeah, I'll do that.