[sr-dev] [tracker] Comment added: modules/debugger: add dbg_pv_dump config function. It dumps on json format the content of pv_cache

[sip-router]

THIS IS AN AUTOMATED MESSAGE, DO NOT REPLY.

The following task has a new comment added:

FS#307 - modules/debugger: add dbg_pv_dump config function. It dumps on json format the content of pv_cache
User who did this - Daniel-Constantin Mierla (miconda)

----------
In first patch, you do pkg_malloc but don't check if you get a non-NULL result. Then you memset and fill the structure.

In second patch, there is a return 0 in the function executed by config, meaning that config file execution will be interrupted (like being an exit) -- probably you wanted to be FALSE in config check, so should be -1 (or other negative value).

Otherwise (once the above are fixed), the patches could be committed -- the part in core in modules are related only to this functionality, not interfering with other code.

But I think cloning all the items in the cache each time you want to use this function is a bit of overhead, although it is just for debugging purposes, thus not expected to be heavily used in production. Therefore I exposed the pv cache via pv_cache_get_table() function, its size is also defined in pvar.h. Now you can walk through the cache from the module. If you decide to go for this option, your first patch is no longer needed, second patch needs to be adapted.

One improvement that I would suggest - add a second parameter (optional) to the new config function to specify the log level -- now it prints to DEBUG, implying that debug=3 must be used.

Also, in the readme, you only pasted the defines for the flags that can be used in the first parameter. You should explain a bit what each is supposed to do.


----------

More information can be found at the following URL:
https://sip-router.org/tracker/index.php?do=details&task_id=307#comment960

You are receiving this message because you have requested it from the Flyspray bugtracking system.  If you did not expect this message or don't want to receive mails in future, you can change your notification settings at the URL shown above.