Re: Added PK_MI_REQ_PRINTV request for MI.

[Mohammad-Reza Nabipoor]

Hello, Kostas.

On Tue, Aug 03, 2021 at 12:29:08AM +0300, Konstantinos Chasialis wrote:
> Hello!
> 
> This patch contains the changes for PK_MI_REQ_PRINTV request.
> 
> All comments are welcome.


Thanks for the patch.


diff --git a/poke/pk-mi.c b/poke/pk-mi.c
index 64670f5c..e88dcc20 100644
--- a/poke/pk-mi.c
+++ b/poke/pk-mi.c
@@ -288,6 +288,37 @@ pk_mi_dispatch_msg (pk_mi_msg msg)
             pk_mi_exit_p = 1;
             break;
           }
+        case PK_MI_REQ_PRINTV:
+          {
+            int success_p, retcode;
+            pk_mi_msg resp;
+            pk_val val, arg;
+            const char *errmsg;

             int ok;

+
+            if (pk_mi_get_num_args (msg) != 1)
+              {
+                success_p = 0;
+                errmsg = "Expected exactly one argument for PRINTV request";
+              }
+            else
+              {
+                arg = pk_mi_get_arg (msg, "value");
+                assert (pk_defvar (poke_compiler, "mi_value", arg) == PK_OK);


Please don't write code inside `assert` macro.
And because this variable is a implementation-detail, please use the prefix
`__pkl_` for it.

                 if (arg == PK_NULL)
                   /* jump to handle error */;
                 ok = pk_defvar (poke_compiler, "__pkl_mi_value", arg);
                 assert (ok == PK_OK);



+                retcode = pk_compile_expression (poke_compiler,
+                                                 "format (\"%v\", mi_value)",
+                                                 NULL, &val);
+                success_p = retcode == PK_OK;
+                if (!success_p)
+                  errmsg = "Invalid value for PRINTV request";


Please change this to an assertion because it's an implementation error (as
we discussed over IRC).


+              }
+
+              resp = pk_mi_make_resp (PK_MI_RESP_PRINTV, pk_mi_msg_number 
(msg),
+                                      success_p, errmsg);
+              pk_mi_set_arg (resp, "string", val);


I think you should set arguments only when there's no error.


+              pk_mi_send (resp);
+              pk_mi_msg_free (resp);
+              break;
+          }
         default:
           assert (0);
         }