Hi YangMin,

On Sun, Jun 29, 2014 at 09:23:12PM +0800, yangmin zhu wrote:
[...]
> 1) I had ever thought about to hide all JSON related details from
> syscall parsers, such as to use functions like
> print_arg_fd(tcp->u_arg[0]);
> and I even implemented a simple framework for that. But I changed not
> to do it in this way because I want to keep the parser code the same
> as the original ones.
> so we can just add some additional code to make it support json
> output. you can see this in sys_read() above, I didn't delete or
> modify any existing code, and I just add some output functions or use
> some wrap macro to support JSON output. And other people could still
> follow the old way to do the output work and do not care about the
> JSON staff.
> 
> 2) Another reason for my choose is that if we hide the details by
> using `print_arg_fmt("%lu", tcp->u_arg[2])` to replace `tprintf(",
> %lu", tcp->u_arg[2])`, we will have to implement two different output
> logical code, one for the original output and one for the JSON output.
> I'm afarid to broke the original format so I prefer to not to modify
> it.
> 
> 3) I also didn't add such specific output functions such as
> `print_arg_fd()` for extensibility. Because printfd() is not only used
> in such sys_* functions, it also used in decode_select(),
> print_dirfd(), printcmsghdr() and so on. If I choose to use spilt all
> the current logical code and the format code of the parser functions.
> I'm afraid it may need much more time and I may couldn't finish it in
> time.

Yes, these are valid points, but I think in the long run we will have to
rewrite parsers in a more structured manner anyway.

> 4) I agree the code is indeed very complicated int my current
> implementation, how do you think the following update:
> int
> sys_read(struct tcb *tcp)
> {
>        if (entering(tcp)) {
>               printfd(tcp, tcp->u_arg[0]);
>               tprints(", ");
>               json_printf("arg sepa");
>       } else {
>               if (syserror(tcp)) {
>                       tprintf("%#lx", tcp->u_arg[1]);
>                       json_printf("arg");
>               }
>               else {
>                       printstr(tcp, tcp->u_arg[1], tcp->u_rval)
>                       json_printf("arg");
>               }
>               tprintf(", %lu", tcp->u_arg[2]);
>               json_printf("arg sepa");
>       }
>       return 0;
> }

Yes, this way it looks less complicated.

In this example, since va_list processing is easier and more efficient
than string parsing, I'd recommend
        json_printf(JSON_ARG, JSON_SEP, 0L);
instead of
        json_printf("arg sepa");

Also, when a function name ends with "printf", it is expected to take a
printf-style format string.  If it doesn't, give it a different name.


-- 
ldv

Attachment: pgpt6Bw6dJ05V.pgp
Description: PGP signature

------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to